Merged #2242 into master.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2242#event-7729582571
You are receiving this because you are subscribed to this thread.
Message ID:
___
Rpm-maint
Looks fine now. I realized just now there was at least one case where the
if-indentation rule was broken in the existing code (the "more than on key"
test), these *fine details* slip past so easily, sigh.
Would be wonderful to have an automated coding style checker on GH.
Anyway, thanks for the
@DemiMarie pushed 3 commits.
01c32b20550ca866869d574e73c45dc6ddf125e5 Avoid type confusion when verifying
signatures
8afe572424b6b6a3526de6373f2b1b51044274b1 Check packet types of signatures and
public keys
d9f6fcb91fdb82b07afdaf1b6e82533755f627c3 Reject multiple PGPTAG_PUBLIC_KEY
packets
I'll review, but you'll want to rebase to clear the unrelated test-suite
failure first.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2242#issuecomment-1298571369
You are receiving this because you are subscribed to this thread.
@DemiMarie pushed 3 commits.
ddb8b20e8e8822045eaf5da33b6ec0cf6361be04 Avoid type confusion when verifying
signatures
f59638fd2e879494ba57fd9f2dc48dfe9e58a90f Check packet types of signatures and
public keys
e0c93993fe673f6bc8103fc87a4e2c6f3f5c38f9 Reject multiple PGPTAG_PUBLIC_KEY
packets
Readability/style issues aside, yes we need to care about fixing segfaults as
long as the internal parser is there. Which might be a while. Thanks for
looking into this!
Here again though, it's really the subkey stuff that is even more brittle. As
the Sequoia option is now in, we could now
@pmatilai commented on this pull request.
> pgpDigAlg sa = sig->alg;
pgpDigAlg ka = key->alg;
- if (sa && sa->verify) {
+ if (sa && sa->verify &&
+ sig->pubkey_algo == key->pubkey_algo) {
Another broken indentation here. A (continued) conditional on the same
@pmatilai commented on this pull request.
> @@ -1150,9 +1153,13 @@ rpmRC pgpVerifySignature(pgpDigParams key,
> pgpDigParams sig, DIGEST_CTX hashctx)
* done all we can, return NOKEY to indicate "looks okay but dunno."
*/
if (key && key->alg) {
+ if (key->tag !=
@pmatilai commented on this pull request.
> @@ -1124,6 +1124,9 @@ rpmRC pgpVerifySignature(pgpDigParams key, pgpDigParams
> sig, DIGEST_CTX hashctx)
if (sig == NULL || ctx == NULL)
goto exit;
+if (sig->tag != PGPTAG_SIGNATURE)
+ goto exit; /* not a signature */
This
@pmatilai commented on this pull request.
> const uint8_t *pend = h + hlen;
int curve = 0;
+if (keyp->tag != PGPTAG_PUBLIC_KEY && keyp->tag != PGPTAG_PUBLIC_SUBKEY)
+ return rc; /* Not a public key */
+if (keyp->alg)
+ return rc; /* We can't handle more than one
@pmatilai commented on this pull request.
> const uint8_t *pend = h + hlen;
int curve = 0;
+if (keyp->tag != PGPTAG_PUBLIC_KEY && keyp->tag != PGPTAG_PUBLIC_SUBKEY)
+ return rc; /* Not a public key */
This isn't in rpm style of commenting: it's stating the obvious, and code
These can cause segfaults; see the included test cases for details. I know the
internal parser is deprecated, but hopefully a segfault-triggering bug is still
worth fixing.
You can view, comment on, or merge this pull request online at:
12 matches
Mail list logo