Patches for aports can be sent to this list

1

[alpine-aports] [PATCH] main/musl: Fix out-of-bound read in sscanf

Marian Buschsieweke
Details
Message ID
<20190325092202.14077-1-marian.buschsieweke@ovgu.de>
Sender timestamp
1553505722
DKIM signature
missing
Download raw message
Patch: +61 -1
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

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äs <timo.teras@iki.fi>
 pkgname=musl
 pkgver=1.1.21
-pkgrel=1
+pkgrel=2
 pkgdesc="the musl c library (libc) implementation"
 url="http://www.musl-libc.org/"
 arch="all"
@@ -16,6 +16,7 @@ esac
 source="http://www.musl-libc.org/releases/musl-$pkgver.tar.gz
 	handle-aux-at_base.patch
 	s390x-fadv.patch
+	sscanf_segfault.patch
 
 	ldconfig
 	__stack_chk_fail_local.c
@@ -146,6 +147,7 @@ compat() {
 sha512sums="fa6c4cc012626c5e517e0e10926fc845e3aa5f863ffaceeb38ac5b9ce0af631a37f6b94f470997db09aa0d5e03f4f28a2db83484b0f98481bea2239c1989d363  musl-1.1.21.tar.gz
 6a7ff16d95b5d1be77e0a0fbb245491817db192176496a57b22ab037637d97a185ea0b0d19da687da66c2a2f5578e4343d230f399d49fe377d8f008410974238  handle-aux-at_base.patch
 e9c9135f6dc3260e62ae6e9c45f3c43574af6ff2c2bfe411eb83f7e80d13bb8c86425cb41fc961e27f7bc15f679db1fbfb267e401bbe81d6cd5b872eb9b1f471  s390x-fadv.patch
+8a5704b27f40d5b8700ba5355538cf16a5d6360e400231648fe1070d5fade9064d412432f62969e44179193283938731e79704575e085b2c7f10020ab977009b  sscanf_segfault.patch
 8d3a2d5315fc56fee7da9abb8b89bb38c6046c33d154c10d168fb35bfde6b0cf9f13042a3bceee34daf091bc409d699223735dcf19f382eeee1f6be34154f26f  ldconfig
 062bb49fa54839010acd4af113e20f7263dde1c8a2ca359b5fb2661ef9ed9d84a0f7c3bc10c25dcfa10bb3c5a4874588dff636ac43d5dbb3d748d75400756d0b  __stack_chk_fail_local.c
 0d80f37b34a35e3d14b012257c50862dfeb9d2c81139ea2dfa101d981d093b009b9fa450ba27a708ac59377a48626971dfc58e20a3799084a65777a0c32cbc7d  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 <dalias@aerifal.cx>
+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=0 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 != 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>=shlim 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
+@@ -22,7 +22,8 @@ int __shgetc(FILE *f)
+ 	off_t cnt = shcnt(f);
+ 	if (f->shlim && cnt >= f->shlim || (c=__uflow(f)) < 0) {
+ 		f->shcnt = f->buf - f->rpos + cnt;
+-		f->shend = 0;
++		f->shend = f->rpos;
++		f->shlim = -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
+@@ -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 != (f)->shend) ? *(f)->rpos++ : __shgetc(f))
+-#define shunget(f) ((f)->shend ? (void)(f)->rpos-- : (void)0)
++#define shunget(f) ((f)->shlim>=0 ? (void)(f)->rpos-- : (void)0)
+ 
+ #define sh_fromstring(f, s) \
+ 	((f)->buf = (f)->rpos = (void *)(s), (f)->rend = (void*)-1)
+-- 
+cgit v1.2.1
+
-- 
2.21.0



---
Unsubscribe:  alpine-aports+unsubscribe@lists.alpinelinux.org
Help:         alpine-aports+help@lists.alpinelinux.org
---
Natanael Copa
Details
Message ID
<20190325131758.61638409@ncopa-macbook.copa.dup.pw>
In-Reply-To
<20190325092202.14077-1-marian.buschsieweke@ovgu.de> (view parent)
Sender timestamp
1553519878
DKIM signature
missing
Download raw message
On Mon, 25 Mar 2019 10:22:02 +0100
Marian Buschsieweke <marian.buschsieweke@ovgu.de> 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äs <timo.teras@iki.fi>
>  pkgname=musl
>  pkgver=1.1.21
> -pkgrel=1
> +pkgrel=2
>  pkgdesc="the musl c library (libc) implementation"
>  url="http://www.musl-libc.org/"
>  arch="all"
> @@ -16,6 +16,7 @@ esac
>  source="http://www.musl-libc.org/releases/musl-$pkgver.tar.gz
>  	handle-aux-at_base.patch
>  	s390x-fadv.patch
> +	sscanf_segfault.patch
>  
>  	ldconfig
>  	__stack_chk_fail_local.c
> @@ -146,6 +147,7 @@ compat() {
>  sha512sums="fa6c4cc012626c5e517e0e10926fc845e3aa5f863ffaceeb38ac5b9ce0af631a37f6b94f470997db09aa0d5e03f4f28a2db83484b0f98481bea2239c1989d363  musl-1.1.21.tar.gz
>  6a7ff16d95b5d1be77e0a0fbb245491817db192176496a57b22ab037637d97a185ea0b0d19da687da66c2a2f5578e4343d230f399d49fe377d8f008410974238  handle-aux-at_base.patch
>  e9c9135f6dc3260e62ae6e9c45f3c43574af6ff2c2bfe411eb83f7e80d13bb8c86425cb41fc961e27f7bc15f679db1fbfb267e401bbe81d6cd5b872eb9b1f471  s390x-fadv.patch
> +8a5704b27f40d5b8700ba5355538cf16a5d6360e400231648fe1070d5fade9064d412432f62969e44179193283938731e79704575e085b2c7f10020ab977009b  sscanf_segfault.patch
>  8d3a2d5315fc56fee7da9abb8b89bb38c6046c33d154c10d168fb35bfde6b0cf9f13042a3bceee34daf091bc409d699223735dcf19f382eeee1f6be34154f26f  ldconfig
>  062bb49fa54839010acd4af113e20f7263dde1c8a2ca359b5fb2661ef9ed9d84a0f7c3bc10c25dcfa10bb3c5a4874588dff636ac43d5dbb3d748d75400756d0b  __stack_chk_fail_local.c
>  0d80f37b34a35e3d14b012257c50862dfeb9d2c81139ea2dfa101d981d093b009b9fa450ba27a708ac59377a48626971dfc58e20a3799084a65777a0c32cbc7d  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 <dalias@aerifal.cx>
> +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=0 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 != 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>=shlim 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 = shcnt(f);
> + 	if (f->shlim && cnt >= f->shlim || (c=__uflow(f)) < 0) {
> + 		f->shcnt = f->buf - f->rpos + cnt;
> +-		f->shend = 0;
> ++		f->shend = f->rpos;
> ++		f->shlim = -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 != (f)->shend) ? *(f)->rpos++ : __shgetc(f))
> +-#define shunget(f) ((f)->shend ? (void)(f)->rpos-- : (void)0)
> ++#define shunget(f) ((f)->shlim>=0 ? (void)(f)->rpos-- : (void)0)
> + 
> + #define sh_fromstring(f, s) \
> + 	((f)->buf = (f)->rpos = (void *)(s), (f)->rend = (void*)-1)
> +-- 
> +cgit v1.2.1
> +



---
Unsubscribe:  alpine-aports+unsubscribe@lists.alpinelinux.org
Help:         alpine-aports+help@lists.alpinelinux.org
---