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

Add a limit to the size of the metadata and checksums files in a gem package. #7568

Conversation

segiddins
Copy link
Member

This is to prevent a malicious gem from causing a denial of service by
including a very large metadata or checksums file,
which is then read into memory in its entirety just by opening the gem package.

This is guaranteed to limit the amount of memory needed, since
gzips (which use deflate streams for compression) have a maximum compression
ratio of 1032:1, so the uncompressed size of the metadata or checksums file
will be at most 1032 times the size of the (limited) amount of data read.

This prevents a gem from causing 500GB of memory to be allocated
to read a 500MB metadata file.

Copy link
Member

@indirect indirect left a comment

Choose a reason for hiding this comment

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

seems like a good plan to me

@simi simi self-requested a review April 11, 2024 08:46
@@ -528,12 +528,13 @@ def normalize_path(pathname)
# Loads a Gem::Specification from the TarEntry +entry+

def load_spec(entry) # :nodoc:
limit = 10*1024*1024
Copy link
Member

Choose a reason for hiding this comment

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

Could become a constant, like MAX_METADATA_BYTES.

Copy link
Member

Choose a reason for hiding this comment

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

🙏

@segiddins segiddins force-pushed the segiddins/add-a-limit-to-the-size-of-the-metadata-and-checksums-files-in-a-gem-package branch from 753d6ac to de55c0a Compare April 15, 2024 05:31
…package.

This is to prevent a malicious gem from causing a denial of service by
including a very large metadata or checksums file,
which is then read into memory in its entirety just by opening the gem package.

This is guaranteed to limit the amount of memory needed, since
gzips (which use deflate streams for compression) have a maximum compression
ratio of 1032:1, so the uncompressed size of the metadata or checksums file
will be at most 1032 times the size of the (limited) amount of data read.

This prevents a gem from causing 500GB of memory to be allocated
to read a 500MB metadata file.
@segiddins segiddins force-pushed the segiddins/add-a-limit-to-the-size-of-the-metadata-and-checksums-files-in-a-gem-package branch from de55c0a to a596e3c Compare April 28, 2024 18:40
lib/rubygems/package.rb Outdated Show resolved Hide resolved
@segiddins segiddins marked this pull request as ready for review April 28, 2024 20:35
Copy link
Member

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

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

Seems like a good solution to me too! Happy to wait for that constant to be extracted or merge this directly.

@deivid-rodriguez
Copy link
Member

Merging since this has had three approvals already. We can extract the constant later.

@deivid-rodriguez deivid-rodriguez merged commit 4842ad9 into master Apr 30, 2024
75 checks passed
@deivid-rodriguez deivid-rodriguez deleted the segiddins/add-a-limit-to-the-size-of-the-metadata-and-checksums-files-in-a-gem-package branch April 30, 2024 15:34
deivid-rodriguez added a commit that referenced this pull request Apr 30, 2024
…ize-of-the-metadata-and-checksums-files-in-a-gem-package

Add a limit to the size of the metadata and checksums files in a gem package.

(cherry picked from commit 4842ad9)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants