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
It causes undefined behavior in multithreaded programs.
You can view, comment on, or merge this pull request online at:
https://github.com/rpm-software-management/rpm/pull/1667
-- Commit Summary --
* Rip out the atexit handler
-- File Changes --
M lib/misc.h (3)
M lib/rpmdb.c (16)
> No you wouldn't. Nobody expects contents of a #-commented line to affect
> anything coming after it, it's just absurd. It's absurd in specs too as
> indicated by the endless bugs and tickets filed on the behavior over the
> years, but at least there we have an excuse or two, and other options
Personally, I would prefer to emit an error here.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
@DemiMarie pushed 2 commits.
267929e21a03398bcf93ae8115a5ef1fe4e3e47c Simplify OpenSSL crypto code
77d5cc99aa2587b60026b7b61f3708b463b09782 Avoid double frees if
EVP_PKEY_assign_RSA fails
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
@DemiMarie pushed 1 commit.
a5d5303bfadd6ebf111ee8b577029ae1d4cfac95 Fix CVE-2021-20249
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
librpm installs an `atexit()` handler to clean up resources, but there is no
protection against other threads that are still using those resources. The
result is undefined behavior. According to [this comment by Rich
@DemiMarie pushed 1 commit.
69519da99e8317f1431c4830c5d6afafa0235020 Fix CVE-2021-20249
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
RPM has been broken on 32-bit platforms since
1efe530450b5bdbd90128327be56c87fa1b6843b, due to an integer promotion
bug: ‘einfo.offset’ gets promoted to unsigned, but promoting a negative
number produces a very large positive number. When that number is
divided by 16, the result is still a large
To handle delta RPMs properly, we need to be able to disable the payload
digest, as for delta RPMs it will be wrong.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
> Fixed via #1640 for a more elaborate commit message + the query.c usage.
> Thanks for spotting and the patch though.
You’re welcome!
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Thanks!
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1640#issuecomment-821233628___
Rpm-maint mailing list
LLDB (which XCode uses) will also work.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/1637#issuecomment-821232630___
Can you post the spec file and a backtrace? Also, if it is not too
inconvenient, would you mind rebuilding with Address Sanitizer and Undefined
Behavior Sanitizer, then re-running the build?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or
If writing to stdout or stderr fails, rpmkeys should exit with a
non-zero status code.
You can view, comment on, or merge this pull request online at:
https://github.com/rpm-software-management/rpm/pull/1633
-- Commit Summary --
* rpmkeys: exit non-zero on I/O errors
-- File Changes --
> @DemiMarie I've repeatedly asked you not to submit more pull-requests of this
> kind, because a large percentage of these "but in theory" patches have only
> introduced regressions despite hours and days wasted trying to review them.
> Rpm relies on this type of arithmetic in any number of
> NAK, this is getting even more confusing.
>
> I totally agree --checksig is far from ideal, but there are LOTS of legacy
> twists associated with it all. I opened #1631 to track it - the main issue to
> me is that it's ambiguous - and it needs to be properly though over, taking
> all the
> While this generally looks good, we probably want to have tests that verify
> positive cases with accepted signatures. Admittedly, I'm not sure how that
> would be wired in...
We already have such tests, and this change does not break them.
--
You are receiving this because you are
Pinging @dmach @Conan-Kudo
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1630#issuecomment-817297754___
Rpm-maint mailing list
`rpmkeys --checksig` exists specifically to verify the signatures on a
package. Therefore, it should imply `--define=_pkgverify_level all` and
`--define=_pkgverify_flags 0x0`. The current behavior is both
counterintuitive and dangerous.
The RPM testsuite relies heavily on controlling the
@DemiMarie pushed 2 commits.
c005dcae33c8fe61d76519b9bcc82d8697ea11ae Simplify comment
8e3fba0bacf0236eedfe5e9a87a5a630b64b0219 Remove useless check
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
In hdrblobVerifyRegion(), negative region offsets were not correctly
rejected, causing an integer overflow.
cc @dmach @Conan-Kudo.
You can view, comment on, or merge this pull request online at:
https://github.com/rpm-software-management/rpm/pull/1628
-- Commit Summary --
* Fix
@Conan-Kudo @dmach
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1612#issuecomment-816892019___
Rpm-maint mailing list
Such packets are probably an attempt to exploit a bug in RPM, rather
than being useful.
You can view, comment on, or merge this pull request online at:
https://github.com/rpm-software-management/rpm/pull/1627
-- Commit Summary --
* Reject extra packets after a signature
-- File Changes --
@DemiMarie pushed 2 commits.
0b3aa0a682951ee90a2e89c9bca91181c7cb0b43 Simplify OpenSSL crypto code
3a334f025056555e846ca52354950e1693bd155f Avoid double frees if
EVP_PKEY_assign_RSA fails
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
@DemiMarie pushed 1 commit.
a668ee79e97ca2ec8c2432be4f63c303d82f1c0d Avoid out-of-bounds pointer
arithmetic in dataLength()
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
Not sure why test 346 failed, but I re-pushed it with just the bare minimal
change.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
This is undefined behavior. Pinging @dmach @Conan-Kudo.
You can view, comment on, or merge this pull request online at:
https://github.com/rpm-software-management/rpm/pull/1626
-- Commit Summary --
* Avoid out-of-bounds pointer arithmetic in dataLength()
-- File Changes --
M
Reopened #1617.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1617#event-4573459838___
Rpm-maint mailing list
Closed #1617.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1617#event-4573459405___
Rpm-maint mailing list
@DemiMarie pushed 0 commits.
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1617/files/bb1a9658bfd45fac52878d53cd1b5abc8569fa39..d1cc512a9c315c56c90c891c5052d3ebfca6b602
@DemiMarie pushed 2 commits.
d1cc512a9c315c56c90c891c5052d3ebfca6b602 Set RPM_MIN_TYPE to 1
bb1a9658bfd45fac52878d53cd1b5abc8569fa39 Revert "Revert "Check that len is in
range before using it""
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
> Good catch. I'm just wondering if we shouldn't instead change RPM_MIN_TYPE to
> 1, because RPM_NULL_TYPE is not actually a type. This kinda confirms that:
>
> https://github.com/rpm-software-management/rpm/blob/ed0e95a21e1d6cd097f25e56e219d07e45e026b1/lib/query.c#L254
My thoughts exactly
We can’t drop it, but we can certainly stop using it internally :)
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
@DemiMarie pushed 1 commit.
2f4d172ac7d291d110d51d4f3b910fce31256696 hdrblobVerifyInfo(): reject trailing
junk
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
`RPM_NULL_TYPE` is used to mean “unknown type”; it is not a valid type
itself. However, `hdrchkType()` did not reject it. Therefore,
`headerPut()` allowed adding an entry of type `RPM_NULL_TYPE` to a header,
which is wrong.
You can view, comment on, or merge this pull request online at:
> I'll investigate how to dig for fingerprints; here is the version with key
> IDs.
Thanks! In addition to the fingerprint vs key ID issue, this still needs a
cryptographic signature check.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or
> Well, it seems it would be helpful to have some advice here. In my local
> setup, packets analysis code detects the following,
> in that order:
>
> ```
>PGPTAG_PUBLIC_KEY; [1] public key id saved
>
>PGPTAG_SIGNATURE
>
> PGPSUBTYPE_SIG_CREATE_TIME
>
@DemiMarie pushed 1 commit.
40443f613f7945df470f1ee6561471d91cda15ce Check that padding is zeroed and
regions are consistent
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
This adds checks that padding is zeroed and that regions are internally
consistent. It also adds a check that the padding is of minimum length.
Fixes #1572.
You can view, comment on, or merge this pull request online at:
https://github.com/rpm-software-management/rpm/pull/1613
-- Commit
> What OpenSSL versions have you tested this with?
Only the one packaged in Fedora 33. I wasn’t able to reproduce the double free
so that part has not been tested. That said, this change should not impose any
new requirements on OpenSSL.
--
You are receiving this because you are subscribed
> Could someone please briefly review two patches above? Thanks.
Revocation signatures are only valid if they are a valid signature of the key
being revoked, and are made by either the key being revoked or a key that it
has designated as valid for revocation.
--
You are receiving this because
> Yeah, I guess we really don't want to read some random memory. Nice catch.
Thanks! For anyone reading this later: this is not a security issue because
the input is trusted.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on
@DemiMarie pushed 1 commit.
77488e885aca87492194959296baaca66f1a3c7c More const correctness
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
See individual commit messages for details.
You can view, comment on, or merge this pull request online at:
https://github.com/rpm-software-management/rpm/pull/1612
-- Commit Summary --
* Remove dead code
* A signature is not a key
* Reject unimplemented critical PGP packets
* Reset
This fixes a double-free bug in the OpenSSL backend. This bug is not
considered exploitable as it cannot be reliably triggered by an attacker. It
also deletes a bunch of cruft.
You can view, comment on, or merge this pull request online at:
@DemiMarie requested changes on this pull request.
Avoid using “whitelist” and properly quote variables containing special
characters
> +cat > "$BUILDINFO" < "$BUILDINFO"
```
This ensures that we properly abort on errors.
> +rpm -qa --queryformat
This is not a problem on GCC with `-fno-strict-overflow`, at least on 64-bit
systems. However, there are several reasons I would like to get this in:
1. It makes the code easier to review. With the code as written, I need to do
additional mental work to determine that it is not exploitable.
> "ecosystem" is nice in that it doesn't limit it to macros/scripts/whatever
> and is pretty future-proof.
>
> The only thing I'm mulling about is the order: another possibility could be
> "rpm-ecosystem-python" so assuming there will be multiple such projects for
> different languages they
@DemiMarie pushed 2 commits.
d52e6bcc338e5945a38539f40c6c296716b08425 Document what RPM expects from the C
compiler
c34b0d67d6becd0923d34cdd642b6216b55fdb0e README: point to some documentation
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
@DemiMarie pushed 1 commit.
bdbbffd0174235fc7395f1f54bafe0db0dd093b1 rpmio: avoid reading past the end of
the mode string
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
> Resolved via #1606 instead, but thanks for the initiative.
You’re welcome!
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
> So the initialization is not the problem? And a fix would be do change the
> for loop to:
>
> ```
> for (; nb && p <= q; p++) {
> ```
The initialization is already undefined behavior.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view
```
/source/lib/fsm.c:304:35: error: format ‘%lu’ expects argument of type ‘long
unsigned int’, but argument 5 has type ‘rpm_loff_t’ {aka ‘long long unsigned
int’} [-Werror=format=]
304 | rpmlog(RPMLOG_DEBUG, " %8s (%s %lu bytes [%d]) %s\n", __func__,
|
It gets decremented again on line 227.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/1602#issuecomment-808072321___
> That is probably undefined behavior in C11 but AIUI not in C99, which rpm
> uses.
It is undefined in C99; see
http://www.open-std.org/jtc1/sc22/wg14/www/C99RationaleV5.10.pdf, §6.3.2.3:
> Regardless how an invalid pointer is created, any use of it yields undefined
> behavior.
So I still
This was found by running the testsuite under `-fsanitize=address`.
You can view, comment on, or merge this pull request online at:
https://github.com/rpm-software-management/rpm/pull/1604
-- Commit Summary --
* rpmio: avoid reading past the end of the mode string
-- File Changes --
M
This was detected by building with GCC’s `-fsanitize=address,pointer-compare`
and running with `ASAN_OPTIONS=detect_invalid_pointer_pairs=1`.
Fixes #1602
You can view, comment on, or merge this pull request online at:
https://github.com/rpm-software-management/rpm/pull/1603
-- Commit Summary
GCC has `-fsanitize=pointer-compare`, which adds instrumentation to detect
invalid pointer comparisons. When built with this flag, and run with
`ASAN_OPTIONS=detect_invalid_pointer_pairs=1`, virtually the entire testsuite
fails due to [an undefined pointer comparison][1] in `rpmio/macro.c`.
> Replaced by #1597. I will not consider any patches in this direction unless
> there's a concrete reproducable bug involved, far too many regressions by
> these theoretical hardening patches.
For clarification: does “undefined behavior detected by sanitizers” qualify?
--
You are receiving
RPM currently does not check that a signature contains exactly one packet.
Requiring that a signature have exactly one packet would reduce the attack
surface of RPM, but would reject packages with multiple signatures. If this is
not possible, we can at least reject signatures that have
Also, the issue isn’t “Clang causes RPM to stop working in normal use”, but
“clang optimizes out critical security checks”. I have no evidence that this
happens in practice, but it could start happening in any minor release.
--
You are receiving this because you are subscribed to this thread.
> OpenMandriva has been shipping RPM compiled by Clang for three years now,
> with optimizations. I would think @berolinux would want to know about
> specific reasons not to do that...
My recommendation to @berolinux would be to pass `-fsanitize=undefined
-fsanitize-minimal-runtime`.
--
You
Right now, I would not be comfortable compiling RPM with any compiler except
GCC unless optimizations were disabled.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
> I'm curious where you encountered a compiler that doesn't do this, but looks
> fine to me.
At least GCC and Clang take advantage of signed integer overflow being
undefined to perform optimizations, unless told not to. And Clang *always*
assumes that dereferencing NULL will never happen, and
> Revocation is one of the many unimplemented things in rpm's OpenPGP support.
>
> In other words, you're not seeing a bug as such, it's just not implemented at
> all, much like expiration is not.
Given the complexity of a full implementation, I wonder if we would be better
off ditching
This adds some documentation I wish I had when researching RPM’s security.
You can view, comment on, or merge this pull request online at:
https://github.com/rpm-software-management/rpm/pull/1599
-- Commit Summary --
* Document what RPM expects from the C compiler
* README: point to some
@DemiMarie pushed 1 commit.
a45799a68e8e281d08266fe9e7c1c87e2e40acc4 Fix some out-of-bounds pointer
arithmetic
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
Commit 22106f5d33628515d22c09c1c15dfd2217535116 assumed that
dataLength() would always return a nonzero number. Unfortunately, that
isn’t the case: dataLength() returns zero for RPM_NULL_TYPE. This meant
that hdrblobVerifyInfo() failed to reject such entries, which are
invalid.
This fixes the
This will break code that defines Lua functions for later use. Is this
intentional? Also, it is still possible to monkeypatch various global tables.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
@DemiMarie pushed 1 commit.
217cde6bf36f4b74da231faf7afdba4310f23b1d Ban empty header tag data entries
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
hdrchkType() incorrectly allowed an entry of type RPM_NULL_TYPE, but
that makes no sense in a header. Also ensure that dataLength() and
hdrblobVerifyInfo() treat empty tag data entries as errors.
You can view, comment on, or merge this pull request online at:
It’s equivalent to `rpmkeys --define _pkgverify_level all --checksig`,
but more convienent and less error-prone.
You can view, comment on, or merge this pull request online at:
https://github.com/rpm-software-management/rpm/pull/1588
-- Commit Summary --
* Add --verifysig flag to rpmkeys
Closed #1492.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1492#event-4482927352___
Rpm-maint mailing list
Thank you!
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1586#issuecomment-802707266___
Rpm-maint mailing list
By using GCC’s overflow-checking builtins, we avoid needing to reason
about overflow manually, which is error-prone. Having GCC do the
arithmetic in infinite signed precision is much nicer.
This was assigned CVE-2021-20249, which is why I am making this PR.
You can view, comment on, or merge
@DemiMarie pushed 1 commit.
a1b3a4582307786ed9e737d7945d1d6fe2a98c50 Enable hardening flags where available
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
> @DemiMarie thank you! What version of lua? I had 5.2.0 and it told me it
> wasn't recent enough. Should I try a more recent one (which one?)
You need 5.4.0, IIRC. 5.3.0 might work.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it
@vsoch Lua is now required unconditionally, and has been _de facto_ required
for a while. Also, GPG is needed to sign packages, but not to verify them.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
> @DemiMarie - interesting. I suppose you have some kind of script to set that
> all up - care to share?
I do! Two of them in fact. One uses podman and another uses bwrap. I also
have the Containerfile I use. I don’t mind sharing them, but be warned that
they have not been cleaned up and
@pmatilai Would a good first step be to make this subject to system security
policy?
--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
> I routinely do a `make check` in Podman containers and never encountered this
> issue. Is there some specific set-up (of the testsuite) that you're referring
> to?
The container image itself is practically empty ― everything is bind-mounted
from the host. To be fair, it is possible that the
@DemiMarie pushed 1 commit.
ecb2a94bc31ca18722f0331c92933af81df59f30 Better detection of I/O errors
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
Done
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1566#issuecomment-799612887___
Rpm-maint mailing list
> > Personally, I would consider being able to disable this on a per-package
> > basis a good idea, but it isn’t a blocker.
>
> How is `--nosignature` failing to achieve that as it is?
It does for `rpm(8)`, but not for `dnf(8)`.
--
You are receiving this because you are subscribed to this
I wholeheartedly agree, which is why I would be perfectly with you requiring
that a compiler support `-fwrapv -fwrapv-pointer -fno-strict-aliasing
-fno-delete-null-pointer-checks` :smile:.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view
@DemiMarie pushed 1 commit.
acfa8f488ae13ccf49f1b004fe0e44d420d93c04 rpmsign: support EdDSA signatures
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
@DemiMarie pushed 2 commits.
aa963fa5121dfe270adad92038b064637a2db0b3 hdrblobInit() needs bounds checks too
e84d135305f8befd2426feaeed21969254c0360f rpmsign: support EdDSA signatures
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
Closed #1503.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1503#event-4453732610___
Rpm-maint mailing list
File signatures are even larger and even less efficient, BTW.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
> Rpm actually already verifies signatures _if present_ by default since 4.0 or
> thereabouts, but it doesn't _require_ them. Enforcing is supported since >=
> 4.14.2 and we also have the bypass-switch (--nosignature) already, so from
> strict technical perspective this is just a matter of one
@DemiMarie pushed 1 commit.
a49f073f15436d0bdea4125dd6189d6ee7e6cac5 Fall back to /tmp if TMPDIR is bogus
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
> It certainly is not an RSA signature. See the discussion in #1202 and related.
> As the multiple signature support work hasn't progressed anywhere (and it
> wont anytime soon), perhaps we should consider just reusing the DSA tag for
> this afterall. I guess that's what @mlschroe originally had
> I'd be more interested in understanding what makes $TMPDIR bogus in this case.
I suspect that it is a fakechroot bug, but I have not yet been able to find a
workaround other than this.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view
> They're not any safer in there than they are in the signature header.
They are if the signature covers them! The problem with them being in the
signature header is that there is no reasonable way to sign these entries. I
would like to be able to cryptographically verify everything that goes
> Sorry, I have no idea what that means, and how it's supposed to guarantee
> anything. The padding doesn't even always exist because it depends on
> alignment.
There will always be between zero and seven bytes of padding (inclusive)
between any two tag data entries, and between the signature
If padding is required to be zeroed, and all entries are required to be sorted,
then parsing and serialization is guaranteed to round-trip successfully.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
> Putting them in the main header would only work if the signing happened
> during the package build. Otherwise, putting anything in the main header
> breaks the immutable region hashes, which is a no-no: signing must not modify
> what is being signed. (IMA signatures were initially this way)
RPM packages contain padding to align header fields and the main header. This
is always zeroed in packages generated by rpm.org, but RPM doesn’t check this.
For RPMv6, we can enforce this from the start.
--
You are receiving this because you are subscribed to this thread.
Reply to this email
Can we move the IMA and fsverity signatures and the sizes into the main header?
One rather annoying problem with the current format is that this data is not
itself signed, so (on a system where IMA and fsverity are turned off) an
attacker can stuff just about anything in those fields without
201 - 300 of 489 matches
Mail list logo
