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.

diff --git a/src/string/memmem.c b/src/string/memmem.c
index 58a21fcd6e8dd36ee0370ad907da635397e912a6..11eff86e443e73d5490124f641ce1653f34f53e4 100644 (file)
--- 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)
 {
 
 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;
        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)
 {
 
 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;
        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 55ba1c7b45a0ef0b7087754684055aa0e84d27ce..43a0207a725be2a04af3b3cc6390a214aaae0995 100644 (file)
--- 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)
 {
 
 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)
 {
        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;
 }
        for (h+=3; *h && hw != nw; hw = hw<<8 | *++h);
        return *h ? (char *)h-3 : 0;
 }