Skip to content

Refactor RPM::File#files #14

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

Merged
merged 2 commits into from
Sep 17, 2022
Merged

Refactor RPM::File#files #14

merged 2 commits into from
Sep 17, 2022

Conversation

jordansissel
Copy link
Owner

Now computes the list of packaged file paths using rpm tags instead of shelling out to cpio -it

Now, file listing is computed from rpm tags DIRNAMES and BASENAMES
instead of piping to cpio.

This also fixes a security issue where a malicious rpm could provide a
custom PAYLOADCOMPRESSOR tag which would allow arbitrary shell
execution.

The security issue was reported by Joern Schneeweisz.

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

I ran the test suite before/after the RPM::File#files refactor and tests path in both old/new code.

@jordansissel jordansissel merged commit c608f86 into master Sep 17, 2022
jordansissel added a commit that referenced this pull request Sep 17, 2022
dirnames tags.

The prior method (in #14) assumed basenames and dirnames had a 1:1 relationship,
but the fpm test suite found this to be an incorrect assumption.

Best I can tell, the relationship is:

* basenames has the filenames
* dirindexes is a list of array index offsets for a given file into the
  dirnames tag.

For example, it seems like the first entry in basenames has a directory
path of dirnames[dirindexes[0]]
@FrancoisCapon
Copy link

I don't found an example of POC for this CVE, so I build one using a Docker container with a test of exploit of the 0.0.11 💀 and the 0.0.12 🛡️:

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.

2 participants