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

Fix check for rubygems vulnerabilities #35

Merged
merged 4 commits into from
May 17, 2024

Conversation

mikesaelim
Copy link
Contributor

@mikesaelim mikesaelim commented May 7, 2024

Welp, I think the check for rubygems vulnerabilities has been broken since February 2021.

Problem

I was looking around the rubysec/ruby-advisory-db repo while working on #34, and I noticed that the /libraries directory that holds the rubygems advisories was gone. Turns out, it was moved to /gems/rubygems-update in July 2019, with a symlink pointing from the old location to the new one, and then the symlink was removed in February 2021. So since then, RubyAudit isn't finding any advisories for any rubygems version.

The good news is, according to the ruby-advisory-db and rubygems's changelog, there doesn't seem to have been any new advisories/CVEs since March 2019 (rubygems v2.7.9 and v3.0.3). So, theoretically, everyone who had kept their rubygems versions up-to-date in 2021 should not have missed out on any advisories.

Solution

I've pointed the rubygems checks at the new home for rubygems advisories, and updated the specs. Updating the specs also required me to update our vendored copy of ruby-advisory-db (that we only use for testing), since the old copy (from 2016) still stores the advisories in /libraries. What do you think?

I'd also like to set up some automatic way to detect this kind of problem if it happens again. We don't control ruby-advisory-db and they have no duty to inform us of changes that may break our specific implementation... and RubyAudit development is very sporadic, so it's hard for its devs to be vigilant for breaking changes. I first thought about a runtime check, but it'd have to error in order to get the user's attention, and they wouldn't be able to do anything about it, so maybe not. Perhaps we can schedule a github CI integration test to run daily/weekly/monthly, which simulates running RubyAudit with the latest ruby-advisory-db on a bunch of ruby and rubygems version that we know have advisories?

@@ -18,28 +18,28 @@
expect(subject.all? do |result|
result.advisory.vulnerable?(result.gem.version)
end).to be_truthy
expect(subject.map { |r| r.advisory.id }).to include('OSVDB-120541')
expect(subject.map { |r| r.advisory.id }).to include('CVE-2015-1855')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are the same advisories - ruby-advisory-db changed their IDs to be CVE-*.

Copy link
Contributor

@leanne73 leanne73 left a comment

Choose a reason for hiding this comment

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

The automatic detection question is interesting. I do think a github integration test is probably the most viable way of doing that. Seems like we should be able to set up something to run every week or month.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it makes sense to update our vendored copy of ruby-advisory-db along with these changes.

@mikesaelim
Copy link
Contributor Author

Okay, I've updated the changelog and this PR should be good to merge if you want. (Looks like I don't have the power to merge in this repo anymore.)

Should we cut a v2.3.1 release? Unfortunately I also don't have access to release anymore.

@mikesaelim
Copy link
Contributor Author

I've set up a weekly integration test that lives in my repo and alerts me if it fails, for now: https://github.com/mikesaelim/ruby_audit_tests

We could eventually migrate that over to this repo, but I figure that I'll iron out the wrinkles in my own repo first, instead of submitting a ton of PRs to fix anything that crops up. 😅

Copy link
Contributor

@leanne73 leanne73 left a comment

Choose a reason for hiding this comment

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

LGTM

Yeah, I think a v2.3.1 release makes sense. I'll merge this and #34 and then cut the release.

@leanne73 leanne73 merged commit 7b2a8a6 into civisanalytics:main May 17, 2024
7 checks passed
@mikesaelim mikesaelim deleted the fix-rubygems-check branch May 25, 2024 02:17
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