From e56f273491cc29a7168e408ebf176a1c209f0368 Mon Sep 17 00:00:00 2001 From: Panu Matilainen Date: Mar 23 2012 13:01:05 +0000 Subject: - fix header data length calculation breakage - fix keyid size bogosity causing breakage on 32bit systems --- diff --git a/rpm-4.9.90-header-datalength.patch b/rpm-4.9.90-header-datalength.patch new file mode 100644 index 0000000..4cdf0b6 --- /dev/null +++ b/rpm-4.9.90-header-datalength.patch @@ -0,0 +1,70 @@ +commit 0b8c3218027c99a6d92c2ca53fe7f42cf87f30a4 +Author: Panu Matilainen +Date: Fri Mar 23 14:17:47 2012 +0200 + + Eliminate broken data end calculation in dataLength() + + - If the caller doesn't know the end pointer, we dont have a whole lot + of chance to come up with a reasonable one either. Just assume + the terminating \0's are there when end boundary is not specified: + when this happens we're dealing with relatively "trusted" data + anyway, the more critical case of reading in unknown headers does + always pass end pointers. + - While capping the end pointer to HEADER_DATA_MAX seems like a + reasonable thing to do (as was done in commit + f79909d04e43cbfbbcdc588530a8c8033c5e0a7c), it doesn't really help + (bad data would likely run past bounds anyway), and it's not right + either: the pointer can be to a stack address, and the stack can be + near the top of addressable range, and ptr + HEADER_DATA_MAX can + cause pointer wraparound. Notably that's exactly what happens + when running 32bit personality process on 64bit system on Linux, + at least in case of i386 process on x86_64, causing all sorts of + breakage.. + +diff --git a/lib/header.c b/lib/header.c +index d741552..023c6e3 100644 +--- a/lib/header.c ++++ b/lib/header.c +@@ -301,16 +301,27 @@ unsigned headerSizeof(Header h, int magicp) + return size; + } + +-/* Bounded header string (array) size calculation, return -1 on error */ ++/* ++ * Header string (array) size calculation, bounded if end is non-NULL. ++ * Return length (including \0 termination) on success, -1 on error. ++ */ + static inline int strtaglen(const char *str, rpm_count_t c, const char *end) + { + const char *start = str; + const char *s; + +- while ((s = memchr(start, '\0', end-start))) { +- if (--c == 0 || s > end) +- break; +- start = s + 1; ++ if (end) { ++ while ((s = memchr(start, '\0', end-start))) { ++ if (--c == 0 || s > end) ++ break; ++ start = s + 1; ++ } ++ } else { ++ while ((s = strchr(start, '\0'))) { ++ if (--c == 0) ++ break; ++ start = s + 1; ++ } + } + return (c > 0) ? -1 : (s - str + 1); + } +@@ -328,8 +339,7 @@ static int dataLength(rpm_tagtype_t type, rpm_constdata_t p, rpm_count_t count, + int onDisk, rpm_constdata_t pend) + { + const char * s = p; +- /* Not all callers supply data end, avoid falling over edge of the world */ +- const char * se = pend ? pend : s + HEADER_DATA_MAX; ++ const char * se = pend; + int length = 0; + + switch (type) { diff --git a/rpm-4.9.90-keyid-size.patch b/rpm-4.9.90-keyid-size.patch new file mode 100644 index 0000000..51bf603 --- /dev/null +++ b/rpm-4.9.90-keyid-size.patch @@ -0,0 +1,37 @@ +commit c5a140133505dbe3cf59c97bbf40c2f5526e5f5b +Author: Panu Matilainen +Date: Thu Mar 22 12:24:55 2012 +0200 + + Oops, "magic eight" is necessary here afterall + + - Fix regression from commit 807b402d95702f3f91e9e2bfbd2b5ca8c9964ed9, + the array gets passed as a pointer (how else would it work at all), + so despite having seemingly correct type, sizeof(keyid) depends + on the pointer size. This happens to be 8 on x86_64 and friends + but breaks on eg i386. + - Also return the explicit size from pgpExtractPubkeyFingerprint(), + this has been "broken" for much longer but then all callers should + really care about is -1 for error. + +diff --git a/rpmio/rpmpgp.c b/rpmio/rpmpgp.c +index 4aac23d..e70cf70 100644 +--- a/rpmio/rpmpgp.c ++++ b/rpmio/rpmpgp.c +@@ -757,7 +757,7 @@ static int getFingerprint(const uint8_t *h, size_t hlen, pgpKeyID_t keyid) + (void) rpmDigestFinal(ctx, (void **)&d, &dlen, 0); + + if (d) { +- memcpy(keyid, (d + (dlen-sizeof(keyid))), sizeof(keyid)); ++ memcpy(keyid, (d + (dlen-8)), 8); + free(d); + rc = 0; + } +@@ -787,7 +787,7 @@ int pgpExtractPubkeyFingerprint(const char * b64pkt, pgpKeyID_t keyid) + if (rpmBase64Decode(b64pkt, (void **)&pkt, &pktlen) == 0) { + if (pgpPubkeyFingerprint(pkt, pktlen, keyid) == 0) { + /* if there ever was a bizarre return code for success... */ +- rc = sizeof(keyid); ++ rc = 8; + } + free(pkt); + } diff --git a/rpm.spec b/rpm.spec index c1be359..4f96669 100644 --- a/rpm.spec +++ b/rpm.spec @@ -22,7 +22,7 @@ Summary: The RPM package management system Name: rpm Version: %{rpmver} -Release: %{?snapver:0.%{snapver}.}5%{?dist} +Release: %{?snapver:0.%{snapver}.}7%{?dist} Group: System Environment/Base Url: http://www.rpm.org/ Source0: http://rpm.org/releases/testing/%{name}-%{srcver}.tar.bz2 @@ -46,6 +46,8 @@ Patch6: rpm-4.9.0-armhfp-logic.patch # Patches already in upstream Patch200: rpm-4.9.90-rpmte-fileinfo.patch Patch201: rpm-4.9.90-rpmte-fileinfo-2.patch +Patch202: rpm-4.9.90-keyid-size.patch +Patch203: rpm-4.9.90-header-datalength.patch # These are not yet upstream Patch301: rpm-4.6.0-niagara.patch @@ -227,6 +229,8 @@ packages on a system. %patch200 -p1 -b .rpmte-fileinfo %patch201 -p1 -b .rpmte-fileinfo-2 +%patch202 -p1 -b .keyid-size +%patch203 -p1 -b .header-datalength %patch301 -p1 -b .niagara %patch302 -p1 -b .geode @@ -453,6 +457,12 @@ exit 0 %doc COPYING doc/librpm/html/* %changelog +* Fri Mar 23 2012 Panu Matilainen - 4.9.90-0.git11505.7 +- fix header data length calculation breakage + +* Thu Mar 22 2012 Panu Matilainen - 4.9.90-0.git11505.6 +- fix keyid size bogosity causing breakage on 32bit systems + * Wed Mar 21 2012 Panu Matilainen - 4.9.90-0.git11505.5 - add temporary fake library provides to get around deltarpm "bootstrap" dependency (yes its dirty)