From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 50828 invoked by alias); 27 Jun 2019 22:46:31 -0000 Mailing-List: contact bzip2-devel-help@sourceware.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Subscribe: List-Id: Sender: bzip2-devel-owner@sourceware.org Received: (qmail 50818 invoked by uid 89); 27 Jun 2019 22:46:30 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Checked: by ClamAV 0.100.3 on sourceware.org X-Virus-Found: No X-Spam-SWARE-Status: No, score=-18.8 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.1 spammy=18000 X-Spam-Status: No, score=-18.8 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on sourceware.org X-Spam-Level: X-HELO: gnu.wildebeest.org Message-ID: <0a2331bc6d0c8500c2c45df1e3ebe01b49ad5831.camel@klomp.org> Subject: Re: bzip2 1.0.7 released From: Mark Wielaard To: bzip2-devel@sourceware.org Cc: Julian Seward Date: Tue, 01 Jan 2019 00:00:00 -0000 In-Reply-To: <20190627205837.GD9273@wildebeest.org> References: <20190627205837.GD9273@wildebeest.org> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Evolution 3.28.5 (3.28.5-2.el7) Mime-Version: 1.0 X-Spam-Flag: NO X-SW-Source: 2019-q2/txt/msg00027.txt.bz2 Hi, A bit more analysis before I go to sleep. On Thu, 2019-06-27 at 22:58 +0200, Mark Wielaard wrote: > On Thu, Jun 27, 2019 at 08:54:08PM +0200, Mark Wielaard wrote: > > * Make sure nSelectors is not out of range (CVE-2019-12900) >=20 > Well, that was quick... There is already a regression report about > this fix. See=20 > https://bugs.launchpad.net/ubuntu/+source/bzip2/+bug/1834494 >=20 > The fix itself is certainly correct: >=20 > diff --git a/decompress.c b/decompress.c > index ab6a624..f3db91d 100644 > --- a/decompress.c > +++ b/decompress.c > @@ -280,21 +280,21 @@ Int32 BZ2_decompress ( DState* s ) > if (uc =3D=3D 1) s->inUse[i * 16 + j] =3D True; > } > makeMaps_d ( s ); > if (s->nInUse =3D=3D 0) RETURN(BZ_DATA_ERROR); > alphaSize =3D s->nInUse+2; >=20=20 > /*--- Now the selectors ---*/ > GET_BITS(BZ_X_SELECTOR_1, nGroups, 3); > if (nGroups < 2 || nGroups > 6) RETURN(BZ_DATA_ERROR); > GET_BITS(BZ_X_SELECTOR_2, nSelectors, 15); > - if (nSelectors < 1) RETURN(BZ_DATA_ERROR); > + if (nSelectors < 1 || nSelectors > BZ_MAX_SELECTORS) > RETURN(BZ_DATA_ERROR); > for (i =3D 0; i < nSelectors; i++) { > j =3D 0; > while (True) { > GET_BIT(BZ_X_SELECTOR_3, uc); > if (uc =3D=3D 0) break; > j++; > if (j >=3D nGroups) RETURN(BZ_DATA_ERROR); > } > s->selectorMtf[i] =3D j; > } >=20 > Because if nSelectors would be > BZ_MAX_SELECTORS it would write over > memory after the selectorMtf array. >=20 > The problem with the file in the report is that it does contain some > nSelectors that are slightly larger than BZ_MAX_SELECTORS. >=20 > The test file can be found here: >=20 https://developer.nvidia.com/embedded/dlc/l4t-jetson-xavier-driver-package-= 31-1-0 >=20 > The fix is simple: >=20 > diff --git a/bzlib_private.h b/bzlib_private.h > index 7975552..ef870d9 100644 > --- a/bzlib_private.h > +++ b/bzlib_private.h > @@ -122,7 +122,7 @@ extern void bz_internal_error ( int errcode ); > #define BZ_G_SIZE 50 > #define BZ_N_ITERS 4 >=20=20 > -#define BZ_MAX_SELECTORS (2 + (900000 / BZ_G_SIZE)) > +#define BZ_MAX_SELECTORS (7 + (900000 / BZ_G_SIZE)) >=20=20 >=20=20 >=20=20 > But of course I cannot tell why increasing the max with 5 is correct. > It might well be that the file is invalid. Before the fix bunzip2 > would overwrite some memory after the selectorMtf array. So it might > be the file decompressed by accident in the past. >=20 > I'll look a but deeper, but if people have a clue what exactly is > going on that would be appreciated. So it looks like some implementations might add more selectors than necessary. For example lbzip2 seems to use a max of 18000 + 1 + 7. Which might explain why our 18002 =3D 2 + (900000 / 50) isn't enough, and why my random increase of 5 seemed to work for the given file. In general the nSelector field can be up to 15 bits, so 32768. So we definitely do want to check the input doesn't overflow (or make BZ_MAX_SELECTORS 32768, but that seems excessive). Cheers, Mark