-
-
Notifications
You must be signed in to change notification settings - Fork 229
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
Proof of concept to check the Bundler version too #299
base: master
Are you sure you want to change the base?
Conversation
I think testing against the bundler version specified in the lockfile makes the most sense, especially as bundler will warn if you use a different version than the lockfile has specified. So, 👍 for me for the concept. Probably will want to add a spec to cover this one-off case. Is that something you could do? @postmodern thoughts? |
My main concern about this suggestion, and the suggestion to test the rubygems version, is that the |
Unfortunately this only goes one way — it will warn you if you use an older version of Bundler than the BUNDLED WITH, but not if it's newer. This would potentially lead to false positives when the "bundled with" version is vulnerable, but not actually the version in use in production. On the other hand, in that scenario it does mean you aren't enforcing that bundler must be the non-vulnerable version, so it's arguably worth knowing about?
Absolutely. Once we nail down the desirable behaviour I'm happy to whip up a more production-ready PR for it.
This is exactly my concern with this approach too. Unfortunately I'm not sure there is any reliable way to check this in all scenarios.
I'm not 100% sure about all the details here, but the BUNDLED WITH does also activate that version of Bundler (the "trampoline"?). I think this only happens by default on Bundler 2.x and higher? (but this might be a Rubygems feature, not Bundler, further complicating the question of "when does this happen?") But if you, for example, have Bundler 2.2.17 and 2.2.18 installed, by default This makes it a little more useful to check, I think, but would be more useful if there was a way to force Bundler to either use the specified version or bail. As far as I'm aware, there is no such option.
As it happens, 2.2.18 does change the lockfile format to fix a vulnerability. For this specific issue (CVE-2020-36327), it actually does seem potentially worth warning about the version in BUNDLED WITH.
It might, and I've been thinking about that possibility a lot as well. I think there's a fairly common setup where this does audit the version of Bundler being run in production: containers. A typical arrangement would be to build an app container image, and then perform the audit on an instance of that. That same image is used to build production containers. So in this case, the version of Bundler encountered by the audit is the same that would be used in production. Of course, that might not give the right answer if the container has 2.2.18 & 2.2.17 installed, and would drop down to 2.2.17 due to the BUNDLED WITH directive. Perhaps the check needs to account for both? The cool thing about checking The cool thing about checking BUNDLED WITH is that it states intent for which version should be activated. Maybe everything is fine on the machine you run the audit check on, because it doesn't have the older, vulnerable version installed that BUNDLED WITH asks for – but then you run the app on a production machine that does still have that old version on, and it activates it due to the BUNDLED WITH directive. I think there's potentially an argument here for checking both, or perhaps making it configurable how to do this check. |
As far as I can tell, CVE-2020-36327 was about how bundler resolved gems to sources internally, not the specific format or encoding of the It also appears the logic around I guess instead of asking whether we should test |
I guess what we really want to test is: which version of Bundler is this project using? Or more accurately: "which version of Bundler will be run when this project runs?" In much the same way that the lockfile answers the question of "which versions of these dependencies will be run?" And that's a very fluffy question when it comes to Bundler itself, because the answer is "that depends which computer you run it on". That might change in the future, but right now it's not an exact science. As you discovered (and my own testing agrees with), the version switching fails open – it will switch if the version is installed, but will continue with whichever version is available if it isn't. I suppose another question that is maybe worth answering (but I'm not so sure about this) is "which version of Bundler was used?". This is something BUNDLED WITH tells us for sure, and if there is concern about dependency resolution-related bugs (e.g. dependency confusion) then perhaps it would be useful to audit that the lockfile wasn't produced by an insecure version, which may have resolved dependencies incorrectly. As a pre-production check that might have some value. As an example, if I use Bundler 2.2.17 to
Whether we want to detect the Bundler code running, or the Bundler code that produced the lockfile, is something that I think varies depending on the precise concern. A dependency confusion attack is likely concerned with which version produced the lockfile, whereas with something like CVE-2019-3881 (storing code insecurely in /tmp/) people would be concerned with which version is running. |
Having thought about this some more, I think the answer is you want to check both. If you build with containers, like many people do, it's actually very possible to run the exact same version of Bundler that will run when you deploy in production: run your production-ready container with a custom entrypoint/command, e.g. But if you're worried about CVE-2020-36327 you also need to check the "BUNDLED WITH" in the lockfile, because if it wasn't generated with a new enough version of Bundler, you're still potentially vulnerable. You need both: the lockfile to be generated by a newer Bundler, and to be interpreted by a newer Bundler. I think it makes sense to check both of these things, or at least to have an option to check both of them. |
Can we revisit this again, especially as GHSA-fj7f-vq84-fh43 just came out? Would love to find ways to let people know they need to update their bundler version. Something (even if partially wrong) is still better than nothing, imho. :-) |
RE GHSA-fj7f-vq84-fh43, we could check the current bundler version ( |
I have no idea if this approach makes sense or not, but I wanted to start a conversation about this:
bundler-audit doesn't audit the version of Bundler itself.
Bundler doesn't appear in the specs in the Gemfile.lock, so if you are using a vulnerable version of Bundler, even if it's listed in the advisory database, bundler-audit won't tell you about it. I think that's a problem.
This might be a terrible approach but this is my shot at doing it, taking the version of Bundler from the "BUNDLED WITH" section in the lockfile. I'm not sure if that makes sense or not. Maybe it would make more sense to try to grab the version of Bundler currently in use? Or maybe the one that created the lockfile is actually the one we care about?
Anyway, I have no attachment to the code whatsoever, but I wanted to start a discussion on what I perceive as an oversight of this otherwise wonderful tool.