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

Only attempt extraction if a known payload compressor is used. #15

Merged
merged 1 commit into from Sep 17, 2022

Conversation

jordansissel
Copy link
Owner

This change is to improve safety primarily to reject malicious rpms with a custom PAYLOADCOMPRESSOR tag causing arr-pm to execute arbitrary code.

The security issue was reported by Joern Schneeweisz.

An alternate implementation could be to do what rpm does in its rpmio library which is to use the C implementations of various compressor libraries. However, doing this in Ruby can cause problems because it's not always possible to compile C Ruby extensions or even install Ruby's FFI library.

Test coverage checks for accepting known payload compressors and also rejecting unknown compressor values.

Context: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/97602

This change is to improve safety primarily to reject malicious rpms
with a custom PAYLOADCOMPRESSOR tag causing arr-pm to execute arbitrary
code.

The security issue was reported by Joern Schneeweisz.

An alternate implementation could be to do what rpm does in its rpmio
library which is to use the C implementations of various compressor
libraries. However, doing this in Ruby can cause problems because it's
not always possible to compile C Ruby extensions or even install Ruby's
FFI library.

Test coverage checks for accepting known payload compressors and also
rejecting unknown compressor values.

Context: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/97602
@jordansissel
Copy link
Owner Author

% bundle exec rspec spec/rpm/file_spec.rb -e compressor -f d
Run options: include {:full_description=>/compressor/}

RPM::File
  #extract
    with an invalid payloadcompressor
      should raise an error
    with a 'gzip' payloadcompressor
      should succeed
    with a 'bzip2' payloadcompressor
bzip2: (stdin) is not a bzip2 file.
cpio: premature end of archive
      should succeed
    with a 'xz' payloadcompressor
xz: (stdin): File format not recognized
cpio: premature end of archive
      should succeed
    with a 'lzma' payloadcompressor
lzma: (stdin): File format not recognized
cpio: premature end of archive
      should succeed
    with a 'zstd' payloadcompressor
      should succeed

Finished in 0.01085 seconds (files took 0.06391 seconds to load)
6 examples, 0 failures

@jordansissel jordansissel merged commit efddd45 into master Sep 17, 2022
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.

None yet

1 participant