git: 56880a501396 - stable/12 - libc: fix undefined behavior from signed overflow in strstr and memmem

Ed Maste emaste at FreeBSD.org
Fri Jan 15 14:26:59 UTC 2021


The branch stable/12 has been updated by emaste:

URL: https://cgit.FreeBSD.org/src/commit/?id=56880a50139647318a123d67fe459ca840351729

commit 56880a50139647318a123d67fe459ca840351729
Author:     Ed Maste <emaste at FreeBSD.org>
AuthorDate: 2020-11-19 00:03:15 +0000
Commit:     Ed Maste <emaste at FreeBSD.org>
CommitDate: 2021-01-15 14:25:35 +0000

    libc: fix undefined behavior from signed overflow in strstr and memmem
    
    unsigned char promotes to int, which can overflow when shifted left by
    24 bits or more. this has been reported multiple times but then
    forgotten. it's expected to be benign UB, but can trap when built with
    explicit overflow catching (ubsan or similar). fix it now.
    
    note that promotion to uint32_t is safe and portable even outside of
    the assumptions usually made in musl, since either uint32_t has rank
    at least unsigned int, so that no further default promotions happen,
    or int is wide enough that the shift can't overflow. this is a
    desirable property to have in case someone wants to reuse the code
    elsewhere.
    
    musl commit: 593caa456309714402ca4cb77c3770f4c24da9da
    
    Obtained from:  musl
    
    (cherry picked from commit 33482dae89c26158a22ccb3b7f2ca6e6652f29b4)
---
 lib/libc/string/memmem.c | 8 ++++----
 lib/libc/string/strstr.c | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/lib/libc/string/memmem.c b/lib/libc/string/memmem.c
index 9e7bf94b1464..be52763e2652 100644
--- a/lib/libc/string/memmem.c
+++ b/lib/libc/string/memmem.c
@@ -41,8 +41,8 @@ twobyte_memmem(const unsigned char *h, size_t k, const unsigned char *n)
 static char *
 threebyte_memmem(const unsigned char *h, size_t k, const unsigned char *n)
 {
-	uint32_t nw = n[0] << 24 | n[1] << 16 | n[2] << 8;
-	uint32_t hw = h[0] << 24 | h[1] << 16 | h[2] << 8;
+	uint32_t nw = (uint32_t)n[0] << 24 | n[1] << 16 | n[2] << 8;
+	uint32_t hw = (uint32_t)h[0] << 24 | h[1] << 16 | h[2] << 8;
 	for (h += 3, k -= 3; k; k--, hw = (hw | *h++) << 8)
 		if (hw == nw)
 			return (char *)h - 3;
@@ -52,8 +52,8 @@ threebyte_memmem(const unsigned char *h, size_t k, const unsigned char *n)
 static char *
 fourbyte_memmem(const unsigned char *h, size_t k, const unsigned char *n)
 {
-	uint32_t nw = n[0] << 24 | n[1] << 16 | n[2] << 8 | n[3];
-	uint32_t hw = h[0] << 24 | h[1] << 16 | h[2] << 8 | h[3];
+	uint32_t nw = (uint32_t)n[0] << 24 | n[1] << 16 | n[2] << 8 | n[3];
+	uint32_t hw = (uint32_t)h[0] << 24 | h[1] << 16 | h[2] << 8 | h[3];
 	for (h += 4, k -= 4; k; k--, hw = hw << 8 | *h++)
 		if (hw == nw)
 			return (char *)h - 4;
diff --git a/lib/libc/string/strstr.c b/lib/libc/string/strstr.c
index d726aff1f1c3..72f8abb9e200 100644
--- a/lib/libc/string/strstr.c
+++ b/lib/libc/string/strstr.c
@@ -40,8 +40,8 @@ twobyte_strstr(const unsigned char *h, const unsigned char *n)
 static char *
 threebyte_strstr(const unsigned char *h, const unsigned char *n)
 {
-	uint32_t nw = n[0] << 24 | n[1] << 16 | n[2] << 8;
-	uint32_t hw = h[0] << 24 | h[1] << 16 | h[2] << 8;
+	uint32_t nw = (uint32_t)n[0] << 24 | n[1] << 16 | n[2] << 8;
+	uint32_t hw = (uint32_t)h[0] << 24 | h[1] << 16 | h[2] << 8;
 	for (h += 2; *h && hw != nw; hw = (hw | *++h) << 8)
 		;
 	return *h ? (char *)h - 2 : 0;
@@ -50,8 +50,8 @@ threebyte_strstr(const unsigned char *h, const unsigned char *n)
 static char *
 fourbyte_strstr(const unsigned char *h, const unsigned char *n)
 {
-	uint32_t nw = n[0] << 24 | n[1] << 16 | n[2] << 8 | n[3];
-	uint32_t hw = h[0] << 24 | h[1] << 16 | h[2] << 8 | h[3];
+	uint32_t nw = (uint32_t)n[0] << 24 | n[1] << 16 | n[2] << 8 | n[3];
+	uint32_t hw = (uint32_t)h[0] << 24 | h[1] << 16 | h[2] << 8 | h[3];
 	for (h += 3; *h && hw != nw; hw = hw << 8 | *++h)
 		;
 	return *h ? (char *)h - 3 : 0;


More information about the dev-commits-src-all mailing list