Control: tags -1 moreinfo
On 2021-06-29 20:15:33 +0300, Peter Pentchev wrote:
> Package: release.debian.org
> Severity: normal
> User: release.debian.org@packages.debian.org
> Usertags: unblock
> X-Debbugs-Cc: team+pkg-rpm@tracker.debian.org
>
> Please unblock package rpm to fix a couple of security problems in
> handling untrusted RPM files.
>
> [ Reason ]
> See #985308 for more information - there are three CVEs filed for
> problems in rpm's parsing of various header fields, one of which
> may even be used to lead to code execution.
>
> [ Impact ]
> If a maliciously-constructed RPM package is installed,
> the RPM database may be corrupted or unexpected code may be
> executed (other than the scriptlets within the RPM file).
>
> [ Tests ]
> None for the present.
>
> [ Risks ]
> The patches are mostly straightforward and extend already-existing
> checks. The only risk here would be rejecting a valid RPM file, but
> from what I understand of the source code, this should not happen
> any more, especially after catching one such case (one of the new
> patches to the Debian package combines three upstream commits, one of
> which relaxes some restrictions for this very reason).
>
> [ Checklist ]
> [x] all changes are documented in the d/changelog
> [x] I reviewed all changes and I approve them
> [x] attach debdiff against the package in testing
>
> unblock rpm/4.16.1.2+dfsg1-2
> diff -Nru rpm-4.16.1.2+dfsg1/debian/changelog rpm-4.16.1.2+dfsg1/debian/changelog
> --- rpm-4.16.1.2+dfsg1/debian/changelog 2021-01-28 12:22:55.000000000 +0200
> +++ rpm-4.16.1.2+dfsg1/debian/changelog 2021-06-29 18:15:25.000000000 +0300
> @@ -1,3 +1,35 @@
> +rpm (4.16.1.2+dfsg1-2) unstable; urgency=medium
> +
> + * Team upload.
> + * Amend the CVE-2021-3421-CVE-2021-20271 patch, add two follow-up
> + upstream commits that relax the restrictions a little bit to
> + allow for some existing RPM packages.
> +
> + -- Peter Pentchev <roam@debian.org> Tue, 29 Jun 2021 18:15:25 +0300
> +
> +rpm (4.16.1.2+dfsg1-1) unstable; urgency=medium
> +
> + * Team upload.
> + * Acknowledge NMUs; thanks, Matthias and Boyuan!
> + * Add the CVE-2021-20266 and CVE-2021-3421-CVE-2021-20271 patches to
> + strengthen the verification of RPM signatures. Closes: #985308
> +
> + -- Peter Pentchev <roam@debian.org> Tue, 29 Jun 2021 14:11:21 +0300
> +
> +rpm (4.16.1.2+dfsg1-0.6) unstable; urgency=medium
> +
> + * Non-maintainer upload.
> + * Only build-depend on gnupg2 on linux architectures.
> +
> + -- Matthias Klose <doko@debian.org> Tue, 09 Mar 2021 23:39:07 +0100
> +
> +rpm (4.16.1.2+dfsg1-0.5) unstable; urgency=medium
> +
> + * Non-maintainer upload.
> + * Only build-depend on libaudit-dev on linux architectures.
> +
> + -- Matthias Klose <doko@debian.org> Tue, 09 Mar 2021 22:50:12 +0100
> +
> rpm (4.16.1.2+dfsg1-0.4) unstable; urgency=medium
>
> * Non-maintainer upload.
> diff -Nru rpm-4.16.1.2+dfsg1/debian/control rpm-4.16.1.2+dfsg1/debian/control
> --- rpm-4.16.1.2+dfsg1/debian/control 2021-01-02 14:03:53.000000000 +0200
> +++ rpm-4.16.1.2+dfsg1/debian/control 2021-06-27 01:21:05.000000000 +0300
> @@ -33,8 +33,8 @@
> liblua5.2-dev,
> libsemanage1-dev [linux-any],
> libgcrypt20-dev,
> - libaudit-dev,
> - gnupg2,
> + libaudit-dev [linux-any],
> + gnupg2 [linux-any],
> Standards-Version: 4.5.1
> Vcs-Browser: https://salsa.debian.org/pkg-rpm-team/rpm
> Vcs-Git: https://salsa.debian.org/pkg-rpm-team/rpm.git
> diff -Nru rpm-4.16.1.2+dfsg1/debian/debugedit.8 rpm-4.16.1.2+dfsg1/debian/debugedit.8
> --- rpm-4.16.1.2+dfsg1/debian/debugedit.8 2021-01-28 12:22:55.000000000 +0200
> +++ rpm-4.16.1.2+dfsg1/debian/debugedit.8 2021-06-27 01:21:05.000000000 +0300
> @@ -1,5 +1,5 @@
> -.\" DO NOT MODIFY THIS FILE! It was generated by help2man 1.47.16.
> -.TH DEBUGEDIT "8" "January 2021" "debugedit 4.16.1.2" "System Administration Utilities"
> +.\" DO NOT MODIFY THIS FILE! It was generated by help2man 1.48.2.
> +.TH DEBUGEDIT "8" "March 2021" "debugedit 4.16.1.2" "System Administration Utilities"
> .SH NAME
> debugedit \- Debuginfo editing helper
> .SH SYNOPSIS
> diff -Nru rpm-4.16.1.2+dfsg1/debian/librpm9.symbols rpm-4.16.1.2+dfsg1/debian/librpm9.symbols
> --- rpm-4.16.1.2+dfsg1/debian/librpm9.symbols 2021-01-02 12:04:09.000000000 +0200
> +++ rpm-4.16.1.2+dfsg1/debian/librpm9.symbols 2021-06-29 12:23:21.000000000 +0300
> @@ -473,3 +473,4 @@
> rpmvsVerify@Base 4.16
> showQueryPackage@Base 4.14.0+dfsg1
> showVerifyPackage@Base 4.14.0+dfsg1
> + xlateTags@Base 4.16.1.2+dfsg1
> diff -Nru rpm-4.16.1.2+dfsg1/debian/patches/CVE-2021-20266.patch rpm-4.16.1.2+dfsg1/debian/patches/CVE-2021-20266.patch
> --- rpm-4.16.1.2+dfsg1/debian/patches/CVE-2021-20266.patch 1970-01-01 02:00:00.000000000 +0200
> +++ rpm-4.16.1.2+dfsg1/debian/patches/CVE-2021-20266.patch 2021-06-29 12:23:21.000000000 +0300
> @@ -0,0 +1,97 @@
> +Description: hdrblobInit() needs bounds checks too
> + Users can pass untrusted data to hdrblobInit() and it must be robust
> + against this.
> +Origin: upstream; https://github.com/rpm-software-management/rpm/commit/8f4b3c3cab8922a2022b9e47c71f1ecf906077ef
> +Author: Demi Marie Obenour <athena@invisiblethingslab.com>
> +Bug-Debian: https://bugs.debian.org/985308
> +Last-Update: 2021-06-27
> +
> +--- a/lib/header.c
> ++++ b/lib/header.c
> +@@ -11,6 +11,7 @@
> + #include "system.h"
> + #include <netdb.h>
> + #include <errno.h>
> ++#include <inttypes.h>
> + #include <rpm/rpmtypes.h>
> + #include <rpm/rpmstring.h>
> + #include "lib/header_internal.h"
> +@@ -1886,6 +1887,25 @@
> + return NULL;
> + }
> +
> ++static rpmRC hdrblobVerifyLengths(rpmTagVal regionTag, uint32_t il, uint32_t dl,
> ++ char **emsg) {
> ++ uint32_t il_max = HEADER_TAGS_MAX;
> ++ uint32_t dl_max = HEADER_DATA_MAX;
> ++ if (regionTag == RPMTAG_HEADERSIGNATURES) {
> ++ il_max = 32;
> ++ dl_max = 64 * 1024 * 1024;
> ++ }
> ++ if (hdrchkRange(il_max, il)) {
> ++ rasprintf(emsg, _("hdr tags: BAD, no. of tags(%" PRIu32 ") out of range"), il);
> ++ return RPMRC_FAIL;
> ++ }
> ++ if (hdrchkRange(dl_max, dl)) {
> ++ rasprintf(emsg, _("hdr data: BAD, no. of bytes(%" PRIu32 ") out of range"), dl);
> ++ return RPMRC_FAIL;
> ++ }
> ++ return RPMRC_OK;
> ++}
> ++
> + rpmRC hdrblobRead(FD_t fd, int magic, int exact_size, rpmTagVal regionTag, hdrblob blob, char **emsg)
> + {
> + int32_t block[4];
> +@@ -1898,13 +1918,6 @@
> + size_t nb;
> + rpmRC rc = RPMRC_FAIL; /* assume failure */
> + int xx;
> +- int32_t il_max = HEADER_TAGS_MAX;
> +- int32_t dl_max = HEADER_DATA_MAX;
> +-
> +- if (regionTag == RPMTAG_HEADERSIGNATURES) {
> +- il_max = 32;
> +- dl_max = 64 * 1024 * 1024;
> +- }
> +
> + memset(block, 0, sizeof(block));
> + if ((xx = Freadall(fd, bs, blen)) != blen) {
> +@@ -1917,15 +1930,9 @@
> + goto exit;
> + }
> + il = ntohl(block[2]);
> +- if (hdrchkRange(il_max, il)) {
> +- rasprintf(emsg, _("hdr tags: BAD, no. of tags(%d) out of range"), il);
> +- goto exit;
> +- }
> + dl = ntohl(block[3]);
> +- if (hdrchkRange(dl_max, dl)) {
> +- rasprintf(emsg, _("hdr data: BAD, no. of bytes(%d) out of range"), dl);
> ++ if (hdrblobVerifyLengths(regionTag, il, dl, emsg))
> + goto exit;
> +- }
> +
> + nb = (il * sizeof(struct entryInfo_s)) + dl;
> + uc = sizeof(il) + sizeof(dl) + nb;
> +@@ -1969,11 +1976,18 @@
> + struct hdrblob_s *blob, char **emsg)
> + {
> + rpmRC rc = RPMRC_FAIL;
> +-
> + memset(blob, 0, sizeof(*blob));
> ++ if (uc && uc < 8) {
> ++ rasprintf(emsg, _("hdr length: BAD"));
> ++ goto exit;
> ++ }
> ++
> + blob->ei = (int32_t *) uh; /* discards const */
> +- blob->il = ntohl(blob->ei[0]);
> +- blob->dl = ntohl(blob->ei[1]);
> ++ blob->il = ntohl((uint32_t)(blob->ei[0]));
> ++ blob->dl = ntohl((uint32_t)(blob->ei[1]));
> ++ if (hdrblobVerifyLengths(regionTag, blob->il, blob->dl, emsg) != RPMRC_OK)
> ++ goto exit;
> ++
> + blob->pe = (entryInfo) &(blob->ei[2]);
> + blob->pvlen = sizeof(blob->il) + sizeof(blob->dl) +
> + (blob->il * sizeof(*blob->pe)) + blob->dl;
> diff -Nru rpm-4.16.1.2+dfsg1/debian/patches/CVE-2021-3421-CVE-2021-20271.patch rpm-4.16.1.2+dfsg1/debian/patches/CVE-2021-3421-CVE-2021-20271.patch
> --- rpm-4.16.1.2+dfsg1/debian/patches/CVE-2021-3421-CVE-2021-20271.patch 1970-01-01 02:00:00.000000000 +0200
> +++ rpm-4.16.1.2+dfsg1/debian/patches/CVE-2021-3421-CVE-2021-20271.patch 2021-06-29 17:06:43.000000000 +0300
> @@ -0,0 +1,180 @@
> +Description: Be much more careful about copying data from the signature header
> + Only look for known tags, and ensure correct type and size where known
> + before copying over. Bump the old arbitrary 16k count limit to 16M limit
> + though, it's not inconceivable that a package could have that many files.
> + While at it, ensure none of these tags exist in the main header,
> + which would confuse us greatly.
> + .
> + This is optimized for backporting ease, upstream can remove redundancies
> + and further improve checking later.
> + .
> + Reported and initial patches by Demi Marie Obenour.
> + .
> + Fixes: RhBug:1935049, RhBug:1933867, RhBug:1935035, RhBug:1934125, ...
> + .
> + Fixes: CVE-2021-3421, CVE-2021-20271
> + .
> + NOTE (Debian): the upstream patch was modified to remove the references to
> + RPMSIGTAG_VERITYSIGNATURES and RPMSIGTAG_VERITYSIGNATUREALGO, which were
> + introduced in upstream changes later than our version.
> + .
> + This Debian patch combines the upstream patch with two follow-up commits:
> + https://github.com/rpm-software-management/rpm/commit/f7b97593af5cf818a5c6c5b9bc55bba6d08c9cb0#diff-5436e6bf58cd803d29ac1261bfb06f30192193674d8b1493a33b34dcce0514a8
> + https://github.com/rpm-software-management/rpm/commit/e2f1f1931c5ccf3ecbe4e1e12cacb1e17a277776#diff-5436e6bf58cd803d29ac1261bfb06f30192193674d8b1493a33b34dcce0514a8
> + ...with the last one fixing a problem with too-strict restrictions in
> + the original change.
> +Origin: upstream; https://github.com/rpm-software-management/rpm/commit/d6a86b5e69e46cc283b1e06c92343319beb42e21
> +Author: Panu Matilainen <pmatilai@redhat.com>
> +Bug-Debian: https://bugs.debian.org/985308
> +Last-Update: 2021-06-29
> +
> +--- a/lib/package.c
> ++++ b/lib/package.c
> +@@ -31,82 +31,78 @@
> + rpmRC rc;
> + };
> +
> ++struct taglate_s {
> ++ rpmTagVal stag;
> ++ rpmTagVal xtag;
> ++ rpm_count_t count;
> ++ int quirk;
> ++} const xlateTags[] = {
> ++ { RPMSIGTAG_SIZE, RPMTAG_SIGSIZE, 1, 0 },
> ++ { RPMSIGTAG_PGP, RPMTAG_SIGPGP, 0, 0 },
> ++ { RPMSIGTAG_MD5, RPMTAG_SIGMD5, 16, 0 },
> ++ { RPMSIGTAG_GPG, RPMTAG_SIGGPG, 0, 0 },
> ++ /* { RPMSIGTAG_PGP5, RPMTAG_SIGPGP5, 0, 0 }, */ /* long obsolete, dont use */
> ++ { RPMSIGTAG_PAYLOADSIZE, RPMTAG_ARCHIVESIZE, 1, 1 },
> ++ { RPMSIGTAG_FILESIGNATURES, RPMTAG_FILESIGNATURES, 0, 1 },
> ++ { RPMSIGTAG_FILESIGNATURELENGTH, RPMTAG_FILESIGNATURELENGTH, 1, 1 },
> ++ { RPMSIGTAG_SHA1, RPMTAG_SHA1HEADER, 1, 0 },
> ++ { RPMSIGTAG_SHA256, RPMTAG_SHA256HEADER, 1, 0 },
> ++ { RPMSIGTAG_DSA, RPMTAG_DSAHEADER, 0, 0 },
> ++ { RPMSIGTAG_RSA, RPMTAG_RSAHEADER, 0, 0 },
> ++ { RPMSIGTAG_LONGSIZE, RPMTAG_LONGSIGSIZE, 1, 0 },
> ++ { RPMSIGTAG_LONGARCHIVESIZE, RPMTAG_LONGARCHIVESIZE, 1, 0 },
> ++ { 0 }
> ++};
Is this constant really supposed to be part of the public ABI? This
looks like it could use a static modifier.
Cheers
> ++
> + /** \ingroup header
> + * Translate and merge legacy signature tags into header.
> + * @param h header (dest)
> + * @param sigh signature header (src)
> ++ * @return failing tag number, 0 on success
> + */
> + static
> +-void headerMergeLegacySigs(Header h, Header sigh)
> ++rpmTagVal headerMergeLegacySigs(Header h, Header sigh, char **msg)
> + {
> +- HeaderIterator hi;
> ++ const struct taglate_s *xl;
> + struct rpmtd_s td;
> +
> +- hi = headerInitIterator(sigh);
> +- for (; headerNext(hi, &td); rpmtdFreeData(&td))
> +- {
> +- switch (td.tag) {
> +- /* XXX Translate legacy signature tag values. */
> +- case RPMSIGTAG_SIZE:
> +- td.tag = RPMTAG_SIGSIZE;
> +- break;
> +- case RPMSIGTAG_PGP:
> +- td.tag = RPMTAG_SIGPGP;
> +- break;
> +- case RPMSIGTAG_MD5:
> +- td.tag = RPMTAG_SIGMD5;
> +- break;
> +- case RPMSIGTAG_GPG:
> +- td.tag = RPMTAG_SIGGPG;
> +- break;
> +- case RPMSIGTAG_PGP5:
> +- td.tag = RPMTAG_SIGPGP5;
> +- break;
> +- case RPMSIGTAG_PAYLOADSIZE:
> +- td.tag = RPMTAG_ARCHIVESIZE;
> +- break;
> +- case RPMSIGTAG_FILESIGNATURES:
> +- td.tag = RPMTAG_FILESIGNATURES;
> +- break;
> +- case RPMSIGTAG_FILESIGNATURELENGTH:
> +- td.tag = RPMTAG_FILESIGNATURELENGTH;
> +- break;
> +- case RPMSIGTAG_SHA1:
> +- case RPMSIGTAG_SHA256:
> +- case RPMSIGTAG_DSA:
> +- case RPMSIGTAG_RSA:
> +- default:
> +- if (!(td.tag >= HEADER_SIGBASE && td.tag < HEADER_TAGBASE))
> ++ for (xl = xlateTags; xl->stag; xl++) {
> ++ /* There mustn't be one in the main header */
> ++ if (headerIsEntry(h, xl->xtag)) {
> ++ /* Some tags may exist in either header, but never both */
> ++ if (xl->quirk && !headerIsEntry(sigh, xl->stag))
> + continue;
> +- break;
> ++ goto exit;
> + }
> +- if (!headerIsEntry(h, td.tag)) {
> +- switch (td.type) {
> +- case RPM_NULL_TYPE:
> +- continue;
> ++ }
> ++
> ++ rpmtdReset(&td);
> ++ for (xl = xlateTags; xl->stag; xl++) {
> ++ if (headerGet(sigh, xl->stag, &td, HEADERGET_RAW|HEADERGET_MINMEM)) {
> ++ /* Translate legacy tags */
> ++ if (xl->stag != xl->xtag)
> ++ td.tag = xl->xtag;
> ++ /* Ensure type and tag size match expectations */
> ++ if (td.type != rpmTagGetTagType(td.tag))
> + break;
> +- case RPM_CHAR_TYPE:
> +- case RPM_INT8_TYPE:
> +- case RPM_INT16_TYPE:
> +- case RPM_INT32_TYPE:
> +- case RPM_INT64_TYPE:
> +- if (td.count != 1)
> +- continue;
> ++ if (td.count < 1 || td.count > 16*1024*1024)
> + break;
> +- case RPM_STRING_TYPE:
> +- case RPM_STRING_ARRAY_TYPE:
> +- case RPM_BIN_TYPE:
> +- if (td.count >= 16*1024)
> +- continue;
> ++ if (xl->count && td.count != xl->count)
> + break;
> +- case RPM_I18NSTRING_TYPE:
> +- continue;
> ++ if (!headerPut(h, &td, HEADERPUT_DEFAULT))
> + break;
> +- }
> +- (void) headerPut(h, &td, HEADERPUT_DEFAULT);
> ++ rpmtdFreeData(&td);
> + }
> + }
> +- headerFreeIterator(hi);
> ++ rpmtdFreeData(&td);
> ++
> ++exit:
> ++ if (xl->stag) {
> ++ rasprintf(msg, "invalid signature tag %s (%d)",
> ++ rpmTagGetName(xl->xtag), xl->xtag);
> ++ }
> ++
> ++ return xl->stag;
> + }
> +
> + /**
> +@@ -378,7 +374,8 @@
> + goto exit;
> +
> + /* Append (and remap) signature tags to the metadata. */
> +- headerMergeLegacySigs(h, sigh);
> ++ if (headerMergeLegacySigs(h, sigh, &msg))
> ++ goto exit;
> + applyRetrofits(h);
> +
> + /* Bump reference count for return. */
> diff -Nru rpm-4.16.1.2+dfsg1/debian/patches/series rpm-4.16.1.2+dfsg1/debian/patches/series
> --- rpm-4.16.1.2+dfsg1/debian/patches/series 2021-01-19 13:28:15.000000000 +0200
> +++ rpm-4.16.1.2+dfsg1/debian/patches/series 2021-06-29 17:03:24.000000000 +0300
> @@ -9,3 +9,5 @@
> 0012-pythondistdeps.py-Use-python3-in-shebang.patch
> debugedit-trunk.diff
> gcc-dwarf5.diff
> +CVE-2021-3421-CVE-2021-20271.patch
> +CVE-2021-20266.patch
--
Sebastian Ramacher
Attachment:
signature.asc
Description: PGP signature