Skip to content

Attempt to automatically generate unaffected_versions: #676

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

Merged
merged 6 commits into from
Jul 9, 2023

Conversation

postmodern
Copy link
Member

Generate unaffected_versions based on the inverse of the first vulnerableVersionRange, iff vulnerableVersionRange starts with > or >= .

* Generate `unaffected_versions` based on the inverse of the first
  `vulnerableVersionRange`, iff `vulnerableVersionRange` starts with
  `> ` or `>= `.
@postmodern postmodern self-assigned this Jul 6, 2023
@postmodern
Copy link
Member Author

@jasnow for your review.

@jasnow
Copy link
Contributor

jasnow commented Jul 6, 2023

FYI: Only focused on "gems" advisories:

Within a single vulnerabilities entry, here is the categories of inputs (<, <=, >=, =, commas,
beta/rc/pre):

Across multiple vulnerabilities entries, are they sort so ".first" works all the time?

How do you want examples of above? GHSA URL, GHSA json, ruby-advisory-db path/filename, sync script output?

@jasnow
Copy link
Contributor

jasnow commented Jul 6, 2023

After I fixed typo, I tested the new versions on the daily GHSA sync files.
Most were "<" and okay, .but other:

ghsa: 8xfw-5q82-3652
#    vulnerableVersionRange: "= 4.1.0"
#    firstPatchedVersion:
#      identifier: 4.1.1
EXPECTED: unaffected_versions: "< 4.1.0"
ACTUAL: no field

ghsa: 6mq2-37j5-w6r6
#    vulnerableVersionRange: "<= 1.3.1"
#    firstPatchedVersion:
#      identifier: 1.4.0
EXPECTED: "> 1.3.1, < 1.4.0"
ACTUAL: no field

@jasnow
Copy link
Contributor

jasnow commented Jul 6, 2023

Another example:

ghsa: f5ww-cq3m-q3g7
......................................................................
INPUT
#  vulnerabilities:
#      name: sanitize
#    vulnerableVersionRange: ">= 3.0.0, < 6.0.2"
#      identifier: 6.0.2
......................................................................
EXPECTED
unaffected_versions:
  - "< 3.0.0"
patched_versions:
  - ">= 6.0.2"
......................................................................
ACTUAL
unaffected_versions:
  - "< 3.0.0, < 6.0.2"
patched_versions:
  - ">= 6.0.2"

@postmodern
Copy link
Member Author

@jasnow I updated the logic to support when the first vulnerableVersionRange starts with = in 239353a.

@postmodern
Copy link
Member Author

@jasnow and added support for parsing composite version ranges (ex: >= 3.0.0, < 6.0.2) in ae07be8.

@jasnow
Copy link
Contributor

jasnow commented Jul 7, 2023

@postmodern
Copy link
Member Author

  • gems/webrick/CVE-2009-4492.yml (ONE LINE, ONCE VALUE) - webrick versions jump from 1.3.1 to 1.4.0.
  • CVE-2019-1335 - I think you mean gems/strong_password/CVE-2019-13354.yml. According to the blog post version 0.0.7 was hijacked. I believe unaffected_versions should actually be < 0.0.7 since version 0.0.7 was the hijacked/malware version.
  • ghsa: 333g-rpr4-7hxq - that's correct, we want to match the lower-bound of the least vulnerableVersionRange.
  • GHSA-7627-mp87-jf6q - ah ha, it appears I do need to sort the vulnerableVersionRanges. It should view < 1.6.0 as the first vulnerableVersionRange.
  • gems/activerecord/CVE-2015-7577.yml - Also needs sorting and should output < 3.1.0.

@jasnow
Copy link
Contributor

jasnow commented Jul 8, 2023

I hope these responses are helpful.

(A) gems/webrick/CVE-2009-4492.yml (ONE LINE, ONCE VALUE) -
webrick versions
jump from 1.3.1 to 1.4.0.

After running the sync script, how do we flag this project to review the documentation to verify that there is no releases between 1.3.1 and 1.4.0.

(B) CVE-2019-1335 - I think you mean gems/strong_password/ CVE-2019-13354.yml. According to the [blog post](https://withatwist.dev/
strong-password-rubygem-hijacked.html) version 0.0.7 was hijacked.
I believe unaffected_versions should actually be < 0.0.7
since version 0.0.7 was the hijacked/malware version.

After running the sync script, how do we flag this project to review the above blog to come to your conclusion.

(C) ghsa: 333g-rpr4-7hxq - that's correct, we want to match
the lower-bound of the least vulnerableVersionRange.

Made a note for my personal reference.

(D) GHSA-7627-mp87-jf6q - ah ha, it appears I do need to sort
the vulnerableVersionRanges. It should view < 1.6.0 as the
first vulnerableVersionRange.

Waiting on fix.

(E) gems/activerecord/CVE-2015-7577.yml - Also needs
sorting and should output < 3.1.0.

Waiting on fix.

@jasnow
Copy link
Contributor

jasnow commented Jul 8, 2023

Tried advisories with [beta, pre, rc, alpha] with operators, such as [~>, >=, <].
Nothing new (UPDATE: found answer: 35 instances to my question. Is this okay for patched_versions field? - ">= 4.0.0.rc4"

FYI: All of these tests are based on real examples (nothing fake).

@postmodern
Copy link
Member Author

I have now added sorting by the first (lower bound) version. Try and see if that resolves the sorting issue.

I hope these responses are helpful.

(A) gems/webrick/CVE-2009-4492.yml (ONE LINE, ONCE VALUE) -
webrick versions
jump from 1.3.1 to 1.4.0.

After running the sync script, how do we flag this project to review the documentation to verify that there is no releases between 1.3.1 and 1.4.0.

Typically vulnerabilities do not disappear and then re-appear, so we will have to trust any gaps in the version numbers are due to minor version bumps (1.3.8 -> 1.4.0).

(B) CVE-2019-1335 - I think you mean gems/strong_password/ CVE-2019-13354.yml. According to the [blog post](https://withatwist.dev/
strong-password-rubygem-hijacked.html) version 0.0.7 was hijacked.
I believe unaffected_versions should actually be < 0.0.7
since version 0.0.7 was the hijacked/malware version.

After running the sync script, how do we flag this project to review the above blog to come to your conclusion.

I believe the code generated the correct unaffected_versions, but the original information was incorrect. I think it's still necessary to double check the GitHub Advisory information against NVD or the original advisory, since there are often mistakes.

(C) ghsa: 333g-rpr4-7hxq - that's correct, we want to match
the lower-bound of the least vulnerableVersionRange.

Made a note for my personal reference.

(D) GHSA-7627-mp87-jf6q - ah ha, it appears I do need to sort
the vulnerableVersionRanges. It should view < 1.6.0 as the
first vulnerableVersionRange.

Waiting on fix.

(E) gems/activerecord/CVE-2015-7577.yml - Also needs
sorting and should output < 3.1.0.

Waiting on fix.

@jasnow
Copy link
Contributor

jasnow commented Jul 9, 2023

Typically vulnerabilities do not disappear and then re-appear, so we will have to
trust any gaps in the version numbers are due to minor version bumps (1.3.8 -> 1.4.0).

Does your comment change if the jump is between 1.3.1 and 1.4.0 for CVE-2009-4492?

@jasnow
Copy link
Contributor

jasnow commented Jul 9, 2023

I believe the code generated the correct unaffected_versions, but the original
information was incorrect. I think it's still necessary to double check the
GitHub Advisory information against NVD or the original advisory, since there are often mistakes.

FYI: My assumption was that this work in for issue 537 (GitHub Action) so the
goal is hands-off. Sounds like this it is more a "dependabot" model of automatically
creating a PR and then someone reviews/approves it.

@jasnow
Copy link
Contributor

jasnow commented Jul 9, 2023

Last night changes fixed two "out of order" vulnerableVersionRange"s listed above ("Waiting on fix").

Therefore if the requirement of this PR is to implement Ruby code to convert GHSA "unaffected_versions"-related data in GHSA format into ruby-advisory-db data format, then I think this PR has done that.

Caverts:

  1. Only GHSA data will be sync'ed into the ruby-advisory-db database with the script.
  2. A manual check will be expected after sync script run.
    -- "I think it's still necessary to double check the GitHub Advisory information against NVD or the original advisory, since there are often mistakes."

Hope this helps.

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.

2 participants