Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 30 Jun 2017 00:35:33 +0300
From: Alexander Monakov <amonakov@...ras.ru>
To: musl@...ts.openwall.com
Subject: [RFC PATCH] fix OOB reads in Xbyte_memmem

Reported by Leah Neukirchen.
---
Presumably the loops were written that way to get good performance even in
absence of loop unrolling and/or without relying on advanced compiler
optimizations. In that case the manual loop pipelining should be kept as is,
and the solution is to reduce iteration count by one, pushing out one test
into the loop epilogue.

Alexander

 src/string/memmem.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/string/memmem.c b/src/string/memmem.c
index 4be6a310..54a66e46 100644
--- a/src/string/memmem.c
+++ b/src/string/memmem.c
@@ -5,27 +5,27 @@
 static char *twobyte_memmem(const unsigned char *h, size_t k, const unsigned char *n)
 {
 	uint16_t nw = n[0]<<8 | n[1], hw = h[0]<<8 | h[1];
-	for (h++, k--; k; k--, hw = hw<<8 | *++h)
-		if (hw == nw) return (char *)h-1;
-	return 0;
+	for (h+=2, k-=2; k; k--, hw = hw<<8 | *h++)
+		if (hw == nw) return (char *)h-2;
+	return hw == nw ? (char *)h-2 : 0;
 }
 
 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;
-	for (h+=2, k-=2; k; k--, hw = (hw|*++h)<<8)
-		if (hw == nw) return (char *)h-2;
-	return 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;
 }
 
 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];
-	for (h+=3, k-=3; k; k--, hw = hw<<8 | *++h)
-		if (hw == nw) return (char *)h-3;
-	return 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;
 }
 
 #define MAX(a,b) ((a)>(b)?(a):(b))
-- 
2.11.0

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.