From 593caa456309714402ca4cb77c3770f4c24da9da Mon Sep 17 00:00:00 2001 From: Rich Felker Date: Thu, 30 Apr 2020 21:36:43 -0400 Subject: [PATCH] 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. --- src/string/memmem.c | 8 ++++---- src/string/strstr.c | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/string/memmem.c b/src/string/memmem.c index 58a21fcd..11eff86e 100644 --- a/src/string/memmem.c +++ b/src/string/memmem.c @@ -12,8 +12,8 @@ static char *twobyte_memmem(const unsigned char *h, size_t k, const unsigned cha 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; return hw == nw ? (char *)h-3 : 0; @@ -21,8 +21,8 @@ static char *threebyte_memmem(const unsigned char *h, size_t k, const unsigned c 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; return hw == nw ? (char *)h-4 : 0; diff --git a/src/string/strstr.c b/src/string/strstr.c index 55ba1c7b..43a0207a 100644 --- a/src/string/strstr.c +++ b/src/string/strstr.c @@ -10,16 +10,16 @@ static char *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; } 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; } -- GitLab