Proof of concept to check the Bundler version too

[zofrex]

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.

[reedloden]

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?

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?

[postmodern]

My main concern about this suggestion, and the suggestion to test the rubygems version, is that the BUNDLED WITH version in the lockfile does not necessarily reflect the version of bundler you or prod might be running. The BUNDLED WITH version only specifies the version of bundler that was used when creating or updating the lockfile. AFAIK, bundler only checks BUNDLED WITH for compatibility reasons, not that you must run the exact same version. Auditing that version would only make sense if bundler introduced a vulnerability into the lockfile (ex: outputting the wrong versions of gems). It might make more sense to audit Bundle::VERSION from within bundler-audit? However, that does not allow us to audit the version of bundler being ran in production; as that's part of the production environment.

[zofrex]

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 — @reedloden

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?

Probably will want to add a spec to cover this one-off case. Is that something you could do?

Absolutely. Once we nail down the desirable behaviour I'm happy to whip up a more production-ready PR for it.

My main concern about this suggestion, and the suggestion to test the rubygems version, is that the BUNDLED WITH version in the lockfile does not necessarily reflect the version of bundler you or prod might be running. — @postmodern

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.

The BUNDLED WITH version only specifies the version of bundler that was used when creating or updating the lockfile. AFAIK, bundler only checks BUNDLED WITH for compatibility reasons, not that you must run the exact same version.

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 bundle will run 2.2.18, but if you run that command with a lockfile that contains "BUNDLED WITH 2.2.17" then it will run 2.2.17, if it is installed. If it is not installed it will run 2.2.18 with no warning.

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.

Auditing that version would only make sense if bundler introduced a vulnerability into the lockfile

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 make more sense to audit Bundle::VERSION from within bundler-audit? However, that does not allow us to audit the version of bundler being ran in production; as that's part of the production environment.

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 Bundler::VERSION is that it will account for the combination of installed versions & BUNDLED WITH. Rubygems will activate the BUNDLED WITH version if it is installed, and the latest if it isn't, so Bundler::VERSION gives a very complete picture of which Bundler will be activated on the machine/image that bundler-audit is running on.

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.

[postmodern]

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 Gemfile.lock.

It also appears the logic around BUNDLED_WITH isn't very strict. I did some experiments locally where I bundle installed a small Gemfile, then edited the BUNDLED_WITH version, then ran bundle exec .//test.rb to see if it would load/run. Downgrading the patch and minor versions worked, even when those versions of bundler were not installed. Changing the BuNDLED_WITH version to versions I did have installed did activate them. Downgrading the major version, did however raise a Gem::GemNotFoundException exception.

I guess instead of asking whether we should test BUNDLED_WITH (or Bundler::VERSION), we should be asking what it is we are testing for?

[zofrex]

I guess instead of asking whether we should test BUNDLED_WITH (or Bundler::VERSION), we should be asking what it is we are testing for?

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 bundle update in my deliberately insecure test project, a successful dependency confusion attack occurs. If I upgrade Bundler to 2.2.18, the issue is not actually resolved until I bundle update again from that version. Even though I'm running 2.2.18, the project is still compromised.

% bundle _2.2.17_ update
...
% bundle _2.2.17_ exec ruby main.rb
Hello from public Gem that is masking the private Gem. Oh no!
% bundle _2.2.18_ install
...
% bundle _2.2.18_ exec ruby main.rb
Hello from public Gem that is masking the private Gem. Oh no!
% bundle _2.2.18_ update
...
% bundle _2.2.18_ exec ruby main.rb
Hello from the private Gem

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.

[zofrex]

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. bundler exec bundler-audit. If you're worried about CVE-2020-36327, you want to make sure you're on a new enough version of Bundler to have the issue patched, so it's useful to check the Bundler version, and possible to do so accurately at least in the scenario where you're using containers.

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.

[reedloden]

Can we revisit this again, especially as https://github.com/rubygems/rubygems/security/advisories/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. :-)

[postmodern]

RE GHSA-fj7f-vq84-fh43, we could check the current bundler version (Bundler::VERSION), since that's the version most likely to encounter a malicious Gemfile. Checking BUNDLED WITH is problematic, as it does not accurately represent the exact version that might be ran in production.