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

Multiple vulnerabilities in RPM #1671

Closed

Conversation

DemiMarie
Copy link
Contributor

Through a combination of manual audits and fuzzing, I found several
vulnerabilities in RPM:

  • RPM does not reject packages that have a signed header, but neither a
    header+payload signature nor a payload digest. Furthermore, rpmkeys -K reports digests signatures OK for such packages. Such a package
    is obviously not validly signed, but RPM nevertheless accepts it.
    This can be mitigated by setting %_pkgverify_level to signature
    or all. I consider it a vulnerability as it violates an assumption
    made by much of the RPM ecosystem: if a package has any signatures,
    RPM will (by default) error out when trying to install it, unless
    the entire package has been properly signed by a trusted key.

  • RPM’s parser for OpenPGP packets has multiple memory unsafety
    issues, including out-of-bounds reads and out-of-bounds pointer
    arithmetic. On 32-bit systems, integer overflows and an infinite
    loop are also possible. It may be possible to use this vulnerability
    to modify a package (that is signed by a trusted key) such that
    it still validates as properly signed, but installing it corrupts
    the RPMDB.

I also found two issues that are not vulnerabilities per se, but which
I still believe should be fixed:

  • RPM accepts signatures that are followed by other OpenPGP packets,
    which are not valid. This opens additional attack surface.

  • RPM does not (obviously) reject signatures that are of an incorrect
    type. I am not sure that they do not wind up being rejected in other
    ways, and even if they are not, I am not sure if this is helpful to
    an attacker. But the fix is trivial, so I included it in the patch.

These vulnerabilities are no longer under embargo as of May 4, 2021. See https://www.openwall.com/lists/oss-security/2021/05/04/2.

DemiMarie and others added 6 commits April 22, 2021 16:02
This deletes some cruft.
Previously, the bignums would be left as dangling and double-freed.
- signatures of the wrong type were accepted
- signatures were allowed to contain multiple packets
- numerous out-of-bounds reads
- undefined pointer arithmetic
This fixes how RPM handles packages that contain a header signature, but
neither header+payload signature nor payload digests.  Such packages are
obviously not properly signed, but RPM previously accepted them.

This could be used to confuse both ‘rpmkeys -K’ and DNF.  Both would
report that the package has been properly signed even when it has not.
The included regression tests demonstrates the change in behavior.
This was used to discover security bugs in the latter.
This allows running the testsuite under UBSAN.
@pmatilai
Copy link
Member

pmatilai commented May 5, 2021

@DemiMarie , I don't understand what you're trying to achieve here. I've explained before that we'll never accept this sort of gigantic pull-request touching multiple unrelated corners in one gulp, and all/most of these patches already exists in separate pull-requests just waiting to be processed.

This multitude of mixed and matched reports and patches here and there are not helping, they are harming.

@pmatilai pmatilai closed this May 5, 2021
@DemiMarie
Copy link
Contributor Author

@DemiMarie , I don't understand what you're trying to achieve here. I've explained before that we'll never accept this sort of gigantic pull-request touching multiple unrelated corners in one gulp, and all/most of these patches already exists in separate pull-requests just waiting to be processed.

Sorry about that. The first two commits are already in #1610 and the last two are of lower importance. Should I split the two remaining ones into separate PRs?

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

2 participants