From 0c9490db6ba0f16877692d246f7e90fd3c42d013 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Thu, 6 May 2021 18:57:19 -0400 Subject: [PATCH 1/2] Simplify bounds check in old-format packet parsing a44f02631adce0c17435d007df847cdcaee816a7 added additional bounds checks to pgpGet(). Use these checks to simplify the old-format packet parsing code by using pgpGet() instead of a manual bounds check. --- rpmio/rpmpgp.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/rpmio/rpmpgp.c b/rpmio/rpmpgp.c index ee5c81e246..533fc8321e 100644 --- a/rpmio/rpmpgp.c +++ b/rpmio/rpmpgp.c @@ -341,7 +341,7 @@ struct pgpPkt { * otherwise -1. */ static int pgpGet(const uint8_t *s, size_t nbytes, const uint8_t *send, - unsigned int *valp) + size_t *valp) { int rc = -1; @@ -373,9 +373,9 @@ static int decodePkt(const uint8_t *p, size_t plen, struct pgpPkt *pkt) } else { /* Old format packet, body length encoding in tag byte */ lenlen = (1 << (p[0] & 0x3)); - if (plen > lenlen) { - pkt->blen = pgpGrab(p+1, lenlen); - } + /* Reject indefinite length packets and check bounds */ + if (pgpGet(p + 1, lenlen, p + plen, &pkt->blen)) + return rc; pkt->tag = (p[0] >> 2) & 0xf; } hlen = lenlen + 1; @@ -581,7 +581,7 @@ static int pgpPrtSig(pgpTag tag, const uint8_t *h, size_t hlen, { uint8_t version = 0; const uint8_t * p; - unsigned int plen; + size_t plen; int rc = 1; if (pgpVersion(h, hlen, &version)) From 76c4d953ee01f38bb5e3d27e3cf14ed50dcaa78a Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Sat, 18 Sep 2021 17:07:19 -0400 Subject: [PATCH 2/2] Add test for OpenPGP packet parsing bug This checks that old-format OpenPGP packets with excessive lengths are rejected. --- tests/Makefile.am | 1 + tests/rpmpgp.at | 11 ++++++++++ tests/rpmpgpcheck.c | 52 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+) create mode 100644 tests/rpmpgp.at create mode 100644 tests/rpmpgpcheck.c diff --git a/tests/Makefile.am b/tests/Makefile.am index b4a2e2e1ce..96921e37d5 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -35,6 +35,7 @@ TESTSUITE_AT += rpmspec.at TESTSUITE_AT += rpmio.at TESTSUITE_AT += rpmorder.at TESTSUITE_AT += rpmvfylevel.at +TESTSUITE_AT += rpmpgp.at EXTRA_DIST += $(TESTSUITE_AT) ## testsuite data diff --git a/tests/rpmpgp.at b/tests/rpmpgp.at new file mode 100644 index 0000000000..0e78675f3f --- /dev/null +++ b/tests/rpmpgp.at @@ -0,0 +1,11 @@ +# rpmpgp.at: rpm OpenPGP parsing test + +AT_BANNER([RPM OpenPGP parsing]) + +# Test OpenPGP parsing +AT_SETUP([[Running tests for malformed OpenPGP packages]]) +AT_KEYWORDS([[rpmkeys digest OpenPGP]]) +AT_CHECK([[ +../../rpmpgpcheck +]],0,) +AT_CLEANUP diff --git a/tests/rpmpgpcheck.c b/tests/rpmpgpcheck.c new file mode 100644 index 0000000000..16c5f99c6e --- /dev/null +++ b/tests/rpmpgpcheck.c @@ -0,0 +1,52 @@ +#include +#include +#include +#include + +#include +#include + +#include + +static unsigned char data_eddsa_asc[] = { + 0x88, 0x75, 0x04, 0x00, 0x16, 0x08, 0x00, 0x1d, 0x16, 0x21, 0x04, 0xe8, + 0x3a, 0x38, 0x0b, 0x85, 0x75, 0x56, 0x2b, 0x3c, 0x6f, 0x41, 0xca, 0x28, + 0xa4, 0x5c, 0x93, 0xb0, 0xb5, 0xb6, 0xe0, 0x05, 0x02, 0x60, 0x0f, 0x77, + 0x1a, 0x00, 0x0a, 0x09, 0x10, 0x28, 0xa4, 0x5c, 0x93, 0xb0, 0xb5, 0xb6, + 0xe0, 0x61, 0x58, 0x00, 0xff, 0x7a, 0xb7, 0xaf, 0xa7, 0x82, 0x3a, 0x1e, + 0xd3, 0x0b, 0x1f, 0xbe, 0xee, 0x50, 0x6c, 0xa0, 0xa7, 0xb1, 0x89, 0x7a, + 0x97, 0x69, 0xb8, 0x6d, 0x84, 0x44, 0x09, 0x77, 0x8c, 0xfa, 0xfe, 0x10, + 0xf6, 0x01, 0x00, 0xcb, 0x53, 0xd9, 0xc5, 0xc1, 0x0e, 0x2c, 0x84, 0x62, + 0x20, 0x65, 0xb0, 0xb9, 0x1a, 0x46, 0xf7, 0xf9, 0xb4, 0xfe, 0xfd, 0x2c, + 0x1e, 0x99, 0x72, 0x58, 0x48, 0xc0, 0x22, 0x63, 0x47, 0x12, 0x0a +}; +static long data_eddsa_asc_len = 119; + +int main(void) +{ + long s = sysconf(_SC_PAGE_SIZE); + assert(s > data_eddsa_asc_len && s < LONG_MAX / 2); + + void *addr = mmap(NULL, 2 * s, PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); + if (addr == MAP_FAILED) { + perror("mmap"); + return 1; + } + if (mprotect((unsigned char *)addr + s, (size_t)s, PROT_NONE)) { + perror("mprotect"); + return 1; + } + memcpy((unsigned char *)addr + (s - data_eddsa_asc_len), data_eddsa_asc, data_eddsa_asc_len); + pgpDigParams params = NULL; + addr += s - data_eddsa_asc_len; + if (pgpPrtParams(addr, data_eddsa_asc_len, PGPTAG_SIGNATURE, ¶ms)) { + fprintf(stderr, "Valid signature not properly accepted\n"); + return 1; + } + data_eddsa_asc[1] += 1; + if (pgpPrtParams(addr, data_eddsa_asc_len, PGPTAG_SIGNATURE, ¶ms) != -1) { + fprintf(stderr, "Invalid signature not properly rejected\n"); + return 1; + } + return 0; +}