X-Original-To: alpine-aports@lists.alpinelinux.org Received: from mx1.tetrasec.net (mx1.tetrasec.net [74.117.190.25]) by lists.alpinelinux.org (Postfix) with ESMTP id 7468FF84D16 for ; Mon, 25 Mar 2019 13:18:04 +0000 (UTC) Received: from mx1.tetrasec.net (mail.local [127.0.0.1]) by mx1.tetrasec.net (Postfix) with ESMTP id DDF669E265F; Mon, 25 Mar 2019 13:18:03 +0000 (UTC) Received: from ncopa-macbook.copa.dup.pw (unknown [194.72.166.2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: alpine@tanael.org) by mx1.tetrasec.net (Postfix) with ESMTPSA id 0CA629E03AE; Mon, 25 Mar 2019 13:18:02 +0000 (UTC) Date: Mon, 25 Mar 2019 13:17:58 +0000 From: Natanael Copa To: Marian Buschsieweke Cc: alpine-aports@lists.alpinelinux.org Subject: Re: [alpine-aports] [PATCH] main/musl: Fix out-of-bound read in sscanf Message-ID: <20190325131758.61638409@ncopa-macbook.copa.dup.pw> In-Reply-To: <20190325092202.14077-1-marian.buschsieweke@ovgu.de> References: <20190325092202.14077-1-marian.buschsieweke@ovgu.de> X-Mailer: Claws Mail 3.17.3 (GTK+ 2.24.32; x86_64-alpine-linux-musl) X-Mailinglist: alpine-aports Precedence: list List-Id: Alpine Development List-Unsubscribe: List-Post: List-Help: List-Subscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On Mon, 25 Mar 2019 10:22:02 +0100 Marian Buschsieweke wrote: > Added patch from commit 8f12c4e110acb3bbbdc8abfb3a552c3ced718039, which > fixes an out-of-bound read in sscanf. > --- > main/musl/APKBUILD | 4 ++- > main/musl/sscanf_segfault.patch | 58 +++++++++++++++++++++++++++++++++ > 2 files changed, 61 insertions(+), 1 deletion(-) > create mode 100644 main/musl/sscanf_segfault.patch This was applied to alpine edge, but apparently the patch does not apply cleanly to musl 1.1.20, so I haven't backported it to alpine 3.9 yet. Are there any bug report or test case related to this patch? Do you know if musl 1.1.20 is affected as well? -nc > diff --git a/main/musl/APKBUILD b/main/musl/APKBUILD > index 935520e46c..69aa8bbb0e 100644 > --- a/main/musl/APKBUILD > +++ b/main/musl/APKBUILD > @@ -2,7 +2,7 @@ > # Maintainer: Timo Ter=C3=A4s > pkgname=3Dmusl > pkgver=3D1.1.21 > -pkgrel=3D1 > +pkgrel=3D2 > pkgdesc=3D"the musl c library (libc) implementation" > url=3D"http://www.musl-libc.org/" > arch=3D"all" > @@ -16,6 +16,7 @@ esac > source=3D"http://www.musl-libc.org/releases/musl-$pkgver.tar.gz > handle-aux-at_base.patch > s390x-fadv.patch > + sscanf_segfault.patch > =20 > ldconfig > __stack_chk_fail_local.c > @@ -146,6 +147,7 @@ compat() { > sha512sums=3D"fa6c4cc012626c5e517e0e10926fc845e3aa5f863ffaceeb38ac5b9ce0= af631a37f6b94f470997db09aa0d5e03f4f28a2db83484b0f98481bea2239c1989d363 mus= l-1.1.21.tar.gz > 6a7ff16d95b5d1be77e0a0fbb245491817db192176496a57b22ab037637d97a185ea0b0d= 19da687da66c2a2f5578e4343d230f399d49fe377d8f008410974238 handle-aux-at_bas= e.patch > e9c9135f6dc3260e62ae6e9c45f3c43574af6ff2c2bfe411eb83f7e80d13bb8c86425cb4= 1fc961e27f7bc15f679db1fbfb267e401bbe81d6cd5b872eb9b1f471 s390x-fadv.patch > +8a5704b27f40d5b8700ba5355538cf16a5d6360e400231648fe1070d5fade9064d412432= f62969e44179193283938731e79704575e085b2c7f10020ab977009b sscanf_segfault.p= atch > 8d3a2d5315fc56fee7da9abb8b89bb38c6046c33d154c10d168fb35bfde6b0cf9f13042a= 3bceee34daf091bc409d699223735dcf19f382eeee1f6be34154f26f ldconfig > 062bb49fa54839010acd4af113e20f7263dde1c8a2ca359b5fb2661ef9ed9d84a0f7c3bc= 10c25dcfa10bb3c5a4874588dff636ac43d5dbb3d748d75400756d0b __stack_chk_fail_= local.c > 0d80f37b34a35e3d14b012257c50862dfeb9d2c81139ea2dfa101d981d093b009b9fa450= ba27a708ac59377a48626971dfc58e20a3799084a65777a0c32cbc7d getconf.c > diff --git a/main/musl/sscanf_segfault.patch b/main/musl/sscanf_segfault.= patch > new file mode 100644 > index 0000000000..629c70e1f6 > --- /dev/null > +++ b/main/musl/sscanf_segfault.patch > @@ -0,0 +1,58 @@ > +From 8f12c4e110acb3bbbdc8abfb3a552c3ced718039 Mon Sep 17 00:00:00 2001 > +From: Rich Felker > +Date: Thu, 14 Mar 2019 20:52:18 -0400 > +Subject: fix crash/out-of-bound read in sscanf > + > +commit d6c855caa88ddb1ab6e24e23a14b1e7baf4ba9c7 caused this > +"regression", though the behavior was undefined before, overlooking > +that f->shend=3D0 was being used as a sentinel for "EOF" status (actual > +EOF or hitting the scanf field width) of the stream helper (shgetc) > +functions. > + > +obviously the shgetc macro could be adjusted to check for a null > +pointer in addition to the !=3D comparison, but it's the hot path, and > +adding extra code/branches to it begins to defeat the purpose. > + > +so instead of setting shend to a null pointer to block further reads, > +which no longer works, set it to the current position (rpos). this > +makes the shgetc macro work with no change, but it breaks shunget, > +which can no longer look at the value of shend to determine whether to > +back up. Szabolcs Nagy suggested a solution which I'm using here: > +setting shlim to a negative value is inexpensive to test at shunget > +time, and automatically re-trips the cnt>=3Dshlim stop condition in > +__shgetc no matter what the original limit was. > +--- > + src/internal/shgetc.c | 3 ++- > + src/internal/shgetc.h | 2 +- > + 2 files changed, 3 insertions(+), 2 deletions(-) > + > +diff --git a/src/internal/shgetc.c b/src/internal/shgetc.c > +index ebd5fae7..a4a9c633 100644 > +--- a/src/internal/shgetc.c > ++++ b/src/internal/shgetc.c > +@@ -22,7 +22,8 @@ int __shgetc(FILE *f) > + off_t cnt =3D shcnt(f); > + if (f->shlim && cnt >=3D f->shlim || (c=3D__uflow(f)) < 0) { > + f->shcnt =3D f->buf - f->rpos + cnt; > +- f->shend =3D 0; > ++ f->shend =3D f->rpos; > ++ f->shlim =3D -1; > + return EOF; > + } > + cnt++; > +diff --git a/src/internal/shgetc.h b/src/internal/shgetc.h > +index 1c30f75f..9435381a 100644 > +--- a/src/internal/shgetc.h > ++++ b/src/internal/shgetc.h > +@@ -26,7 +26,7 @@ hidden int __shgetc(FILE *); > + #define shcnt(f) ((f)->shcnt + ((f)->rpos - (f)->buf)) > + #define shlim(f, lim) __shlim((f), (lim)) > + #define shgetc(f) (((f)->rpos !=3D (f)->shend) ? *(f)->rpos++ : __shget= c(f)) > +-#define shunget(f) ((f)->shend ? (void)(f)->rpos-- : (void)0) > ++#define shunget(f) ((f)->shlim>=3D0 ? (void)(f)->rpos-- : (void)0) > +=20 > + #define sh_fromstring(f, s) \ > + ((f)->buf =3D (f)->rpos =3D (void *)(s), (f)->rend =3D (void*)-1) > +--=20 > +cgit v1.2.1 > + --- Unsubscribe: alpine-aports+unsubscribe@lists.alpinelinux.org Help: alpine-aports+help@lists.alpinelinux.org ---