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

Removed 3 lines in lib/github_advisory_sync.rb script to get rid of unneeded debug text #637

Closed
wants to merge 1 commit into from

Conversation

jasnow
Copy link
Contributor

@jasnow jasnow commented Jun 18, 2023

Removed 3 lines in lib/github_advisory_sync.rb script to get rid of unneeded debug text.
This change is needed as part of #537.

The 3 lines are:

file.write "\n\n# GitHub advisory data below - **Remove this data before committing**\n"
file.write "# Use this data to write patched_versions (and potentially unaffected_versions) above\n"
file.write advisory.merge("vulnerabilities" => vulnerabilities).to_yaml

Here is a sample of the above script lines:

(a blank lines)

# GitHub advisory data below - **Remove this data before committing**
# Use this data to write patched_versions (and potentially unaffected_versions) above
---
identifiers:
- type: GHSA
  value: GHSA-8qrh-h9m2-5fvf
- type: CVE
  value: CVE-2009-3009
summary: Moderate severity vulnerability that affects rails
description: Cross-site scripting (XSS) vulnerability in Ruby on Rails 2.x before
  2.2.3, and 2.3.x before 2.3.4, allows remote attackers to inject arbitrary web script
  or HTML by placing malformed Unicode strings into a form helper.
severity: MODERATE
cvss:
  score: 0.0
  vectorString: 
references:
- url: https://nvd.nist.gov/vuln/detail/CVE-2009-3009
- url: https://exchange.xforce.ibmcloud.com/vulnerabilities/53036
- url: https://github.com/advisories/GHSA-8qrh-h9m2-5fvf
- url: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=545063
- url: http://groups.google.com/group/rubyonrails-security/msg/7f57cd7794e1d1b4?dmode=source
- url: http://lists.apple.com/archives/security-announce/2010//Mar/msg00001.html
- url: http://lists.opensuse.org/opensuse-security-announce/2009-10/msg00004.html
- url: http://secunia.com/advisories/36600
- url: http://secunia.com/advisories/36717
- url: http://securitytracker.com/id?1022824
- url: http://support.apple.com/kb/HT4077
- url: http://weblog.rubyonrails.org/2009/9/4/xss-vulnerability-in-ruby-on-rails
- url: http://www.debian.org/security/2009/dsa-1887
- url: http://www.osvdb.org/57666
- url: http://www.securityfocus.com/bid/36278
- url: http://www.vupen.com/english/advisories/2009/2544
- url: https://github.com/rubysec/ruby-advisory-db/blob/master/gems/activesupport/CVE-2009-3009.yml
publishedAt: '2017-10-24T18:33:38Z'
withdrawnAt: 
vulnerabilities:
- package:
    name: rails
    ecosystem: RUBYGEMS
  vulnerableVersionRange: ">= 2.3.0, < 2.3.4"
  firstPatchedVersion:
    identifier: 2.3.4
- package:
    name: rails
    ecosystem: RUBYGEMS
  vulnerableVersionRange: ">= 2.0.0, < 2.2.3"
  firstPatchedVersion:
    identifier: 2.2.3

@reedloden

@postmodern
Copy link
Member

The debug lines should stay, as they instruct the user to replace vulnerabilities with patched_versions. Ideally if we could convert the GHSA vulnerabilities version ranges into rubygems version ranges, we wouldn't need the debug text or the temporary vulnerabilities data.

@postmodern postmodern closed this Jun 18, 2023
@jasnow
Copy link
Contributor Author

jasnow commented Jun 18, 2023

I duplicated those lines in #570 so this whole section could be ignored during my daily runs.

@jasnow jasnow deleted the rm-sync-debug-text branch June 18, 2023 22:47
@postmodern
Copy link
Member

I ended up replacing patched_versions with placeholder data. See 0ab6731.

@jasnow
Copy link
Contributor Author

jasnow commented Jun 19, 2023

How about this compromise? "patched_versions" => ['FIX ME', vulnerabilities],

@postmodern
Copy link
Member

I would prefer to either try to parse the vulnerableVersionRange or leave patched_versions with a placeholder value.

@postmodern
Copy link
Member

I wonder if we could derive patched_versions by firstPatchedVersion. They could be converted to ~> x.y.z, where the first or largest version would be converted to >= x.y.z. If the last vulnerableVersionRange starts with a >= x.y.z, then that implies an unaffected_versions.

@jasnow
Copy link
Contributor Author

jasnow commented Jun 19, 2023

I wonder if we could derive patched_versions by firstPatchedVersion. They could be converted to ~> x.y.z, where the first or largest version would be converted to >= x.y.z. If the last vulnerableVersionRange starts with a >= x.y.z, then that implies an unaffected_versions.

I looking forward to seeing how you solve "derive the patched_versions and unaffected_versions fields" issue.

@postmodern
Copy link
Member

@jasnow random question, is firstPatchedVersion/identifier always in descending order?

@jasnow
Copy link
Contributor Author

jasnow commented Jun 20, 2023

@jasnow random question, is firstPatchedVersion/identifier always in descending order?

I do not know. I never focus of what GHSA format said, just what I need to convert to your format.

Here is one example:

gems/redcloth/CVE-2012-6684.yml-vulnerabilities:
gems/redcloth/CVE-2012-6684.yml-- package:
gems/redcloth/CVE-2012-6684.yml-    name: redcloth
gems/redcloth/CVE-2012-6684.yml-    ecosystem: RUBYGEMS
gems/redcloth/CVE-2012-6684.yml-  vulnerableVersionRange: "< 4.3.0"
gems/redcloth/CVE-2012-6684.yml:  firstPatchedVersion:
gems/redcloth/CVE-2012-6684.yml-    identifier: 4.3.0

Converts to (omit unaffected_versions: field):

patched_version:
  -- ">= 4.3.0"

@jasnow
Copy link
Contributor Author

jasnow commented Jul 10, 2023

@postmodern - May want to review this change after the #570, #641, #648, #649, and #676 work.

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