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

Increase the maximum unpacked size of a crate to 1GiB (CVE-2022-36114 mitigation) #11151

Closed
wants to merge 1 commit into from

Conversation

tux3
Copy link

@tux3 tux3 commented Sep 27, 2022

Hello, we host a crate on a private registry that includes a vendored static C++ library supporting several targets (mobiles and desktop on various architectures). Some releases of the crate have been above 512MB, as the vendored size keeps scaling with the number of supported targets.

With the latest stable cargo, CI is broken for our users due to the zip-bomb detection triggering on previous releases above 512MiB.
The commit message with more rationale for adjusting the zip-bomb threshold follows.


A use-case of private registries is to host projects that may require slightly higher limits than what is a good fit for the public crates.io registry, such as crates with vendored binary dependencies, or other larger crates that may not fit on crates.io.

When supporting many targets (architecture/OS combinations), the size of a crate on a private registry with vendored dependencies is proportional in the number of targets, so as support for more targets is added, a crate can reasonably exceed 512MiB.

The limit to pick is somewhat arbitrary, but in the context of CVE-2022-36114 we should be considerate of users with lower-end or older hardware, who may be the most vulnerable to zip-bombs.

As a comparison point, on my system building a hello world with reqwest that GETs example.org and prints the reply consumes 538M under the target/ folder.
Projects beyond hello worlds that include crates from private registries would usually require on the order of 1-10 GiBs free disk space for the target folder.

It seems resonable to pick a threshold for treating a Rust crate in an external registry as a zip bomb up to be in the same order of magnitude (or just above) disk usage numbers that a regular user could be expected to encounter in normal, non-malicious scenarios.

This commit only bumps the limit to 1GiB based on need, but it may not be entirely unreasonable in the future to consider even slightly higher thresholds to not be zip-bombs, on a "pro re nata" (as needed) basis.

A common use-case of private registries is to host projects that
may require slightly higher limits than what is a good fit for the
public crates.io registry, such as crates with vendored binary
dependencies, or other larger crates that may not fit on crates.io.

When supporting many targets (architecture/OS combinations),
the size of a crate on a private registry with vendored dependencies
is proportional in the number of targets, so as support for more
targets is added, a crate can reasonably exceed 512MiB.

The limit to pick is somewhat arbitrary, but in the context of
CVE-2022-36114 we should be considerate of users with lower-end or
older hardware, who may be the most vulnerable to zip-bombs.

As a comparison point, on my system building a hello world
with `reqwest` that GETs example.org and prints the reply
consumes 538M under the target/ folder.
Projects beyond hello worlds that include crates from private
registries would usually require on the order of 1-10 GiBs free disk
space for the target folder.

It seems resonable to pick a threshold for treating a Rust crate
in an external registry as a zip bomb up to be in the same order of
magnitude (or just above) disk usage numbers that a regular user
could be expected to encounter in normal, non-malicious scenarios.

This commit only bumps the limit to 1GiB based on need,
but it may not be entirely unreasonable in the future to consider even
slightly higher thresholds to not be zip-bombs,
on a "pro re nata" (as needed) basis.
@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @epage (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 27, 2022
@tux3
Copy link
Author

tux3 commented Sep 27, 2022

(cc @joshtriplett @pietroalbini since I think you worked on the previous patch: #11089)

Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

I feel like it might deserve a configurable setting to manage this. Just not sure where the best place is for it.

[registries.<name>] # put it here?
max_unpack_size = "?" # in bytes?, accept SI and/or IEC format?

Zulip thread discussion: https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/.2311151.20configurable.20unpack.20size.3F

@bjorn3
Copy link
Member

bjorn3 commented Oct 12, 2022

Would it make sense to use a separate crate for each target? This is what winapi does. The advantage of this is that you only have to download the crates for targets that you actually compile for rather than download the full >512MiB.

@tux3
Copy link
Author

tux3 commented Oct 13, 2022

I think splitting by target should be possible in our case, good idea!
That seems like a net positive optimization (even if we weren't running into the unpack limit).

I also like max_unpack_size, separately from the crate splitting work. On a purely subjective level, sometimes it's hard to pick software limits that work for everyone, and they can feel a little arbitrary as a result.
It feels nice to have a recourse when a computer refuses to do something that it thinks you didn't really want or understand. I think having either a configurable limit or a broader -yes-i-know-what-i-am-doing type of flag helps give agency to the user.

It's not too uncommon for me to run a cargo command that ends up filling the rest of the drive, as just a regular build can consume a lot of space on my laptop. That said, for some people running out of space unexpectedly can be a serious problem, so the mitigation does something useful and beneficial.
However, my understanding is that this mitigation only helps users who want to fetch the crate, but not build (as that would run the build script, which can carry out the same attack). I also suspect a large majority of users live in an environment where zip-bomb attacks are, relatively, very rare (compared to e.g. ransomware, phishing, etc).

So from a cost/benefit point of view, considering false positives, the rarity of the attack scenario in the first place, and the impact of the attack, I think having any sort of option or tunable would be very helpful. I know that for me, as someone who occasionally experiences ENOSPC situations anyways, it's not necessarily helpful to me when the heuristic takes the final decision.

Another possibility, outside of adding a max_unpack_size setting, or a sort of --keep-going flag, or raising the threshold of the heuristic, would be to ask the user interactively. Though I'm not sure how much work that would take, and non-interactive users would probably want an option as well.
I've no strong preference towards any of these, I'd be happy as long as there's any way at all to express my intent when the computer says no =)

@weihanglo
Copy link
Member

@tux3, I've created a PR #11337 respecting compression ratio. Please see if it meets your needs. Thank you!

@weihanglo
Copy link
Member

Hello. I am going to close this as it is superseded by #11337. Let's wait for responses from the team. Thank you all for the input so far!

@weihanglo weihanglo closed this Nov 24, 2022
@tux3
Copy link
Author

tux3 commented Nov 24, 2022

Sounds good to me. I was going to close this one on the merge of #11337, but this is fine too. Thank you for driving #11337, it's really helpful =)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants