Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cvescan: Exclude fails by version #18731

Merged
merged 3 commits into from
Oct 27, 2021
Merged

cvescan: Exclude fails by version #18731

merged 3 commits into from
Oct 27, 2021

Conversation

phlax
Copy link
Member

@phlax phlax commented Oct 22, 2021

Signed-off-by: Ryan Northey [email protected]

Commit Message:
Additional Description:

This doesnt fix the cve scanner, but does exclude some errors/warnings that have already been updated

We still have this

  CVE ID: CVE-2021-22930
  CVSS v3 score: 9.8
  Severity: CRITICAL
  Published date: 2021-10-07
  Last modified date: 2021-10-18
  Dependencies: com_github_nodejs_http_parser
  Description: Node.js before 16.6.0, 14.17.4, and 12.22.4 is vulnerable to a use
  after free attack where an attacker might be able to exploit the
  memory corruption, to change process behavior.
  Affected CPEs:
  - cpe:2.3:a:nodejs:node.js:*

here https://dev.azure.com/cncf/envoy/_build/results?buildId=92610&view=logs&j=12f1170f-54f2-53f3-20dd-22fc7dff55f9&t=9c939e41-62c2-5605-5e05-fc3554afc9f5&l=116

iirc the proposed solution to that one is to replace the parser altogether

Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

@phlax
Copy link
Member Author

phlax commented Oct 22, 2021

cc @moderation @htuch

@moderation
Copy link
Contributor

Switch away from deprecated http_parser to llhttp is being tracked at:

@phlax
Copy link
Member Author

phlax commented Oct 22, 2021

grrr coverage....

lill kick

@phlax
Copy link
Member Author

phlax commented Oct 25, 2021

im tempted to think that the node.js bug is not related to the http parser, and can probably added to the exclude list

@@ -80,6 +80,15 @@
# Tracking issue to fix versioning:
# https://github.com/envoyproxy/envoy/issues/18354
'CVE-2021-38153',
# Excluded by version
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you leave a slightly more verbose message, i.e. "These are false positives triggered by the inability of cvescan to accurately compute vesion" or something like that? Thanks. Otherwise LGTM.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, i have improved the message, and merged the immediately preceding exclusion, which was added for the same reason, and had a ~duplicating message

@htuch htuch self-assigned this Oct 25, 2021
Signed-off-by: Ryan Northey <[email protected]>
@phlax
Copy link
Member Author

phlax commented Oct 26, 2021

@htuch shall i add the nodejs cve (posted in PR comment) to the exclusions ?

htuch
htuch previously approved these changes Oct 26, 2021
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading https://bugzilla.redhat.com/show_bug.cgi?id=1988394, I think the Node.js issue is on their side, rather than nghttp2, so we should allowlist that one.

@phlax
Copy link
Member Author

phlax commented Oct 26, 2021

ill add it this PR now...

Signed-off-by: Ryan Northey <[email protected]>
@htuch htuch merged commit e8d2b1e into envoyproxy:main Oct 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants