commit 9dd115d6b59aaa7d2f444efb9d7992e472fbfb0f Author: Nick Mathewson nickm@torproject.org Date: Wed Dec 11 15:14:43 2013 -0500
Another 10363 instance: this one in tor_memmem fallback code --- changes/bug10363 | 3 +++ src/common/compat.c | 20 ++++++++++++++------ src/test/test_util.c | 4 ++++ 3 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/changes/bug10363 b/changes/bug10363 index cf5b7fd..a3c9905 100644 --- a/changes/bug10363 +++ b/changes/bug10363 @@ -3,4 +3,7 @@ that could, under unlucky circumstances, have led to a pointer overflow. Fixes bug #10363; bugfixes on 0.2.0.10-alpha and 0.2.3.6-alpha. Reported by "bobnomnom". + - Fix another possibly undefined pointer operations in tor_memmem + fallback implementation. Another case of bug #10363; bugfix on + 0.1.1.1-alpha.
diff --git a/src/common/compat.c b/src/common/compat.c index d88c5f9..9b8a0a4 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -498,21 +498,29 @@ tor_memmem(const void *_haystack, size_t hlen, #else /* This isn't as fast as the GLIBC implementation, but it doesn't need to * be. */ - const char *p, *end; + const char *p, *last_possible_start; const char *haystack = (const char*)_haystack; const char *needle = (const char*)_needle; char first; tor_assert(nlen);
+ if (nlen > hlen) + return NULL; + p = haystack; - end = haystack + hlen; + /* Last position at which the needle could start. */ + last_possible_start = haystack + hlen - nlen; first = *(const char*)needle; - while ((p = memchr(p, first, end-p))) { - if (p+nlen > end) - return NULL; + while ((p = memchr(p, first, last_possible_start + 1 - p))) { if (fast_memeq(p, needle, nlen)) return p; - ++p; + if (++p > last_possible_start) { + /* This comparison shouldn't be necessary, since if p was previously + * equal to last_possible_start, the next memchr call would be + * "memchr(p, first, 0)", which will return NULL. But it clarifies the + * logic. */ + return NULL; + } } return NULL; #endif diff --git a/src/test/test_util.c b/src/test/test_util.c index 65d9d2f..bf87db7 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -1045,6 +1045,10 @@ test_util_strmisc(void) test_assert(!tor_memmem(haystack, 4, "cde", 3)); haystack = "ababcad"; test_eq_ptr(tor_memmem(haystack, 7, "abc", 3), haystack + 2); + test_eq_ptr(tor_memmem(haystack, 7, "ad", 2), haystack + 5); + test_eq_ptr(tor_memmem(haystack, 7, "cad", 3), haystack + 4); + test_assert(!tor_memmem(haystack, 7, "dadad", 5)); + test_assert(!tor_memmem(haystack, 7, "abcdefghij", 10)); /* memstr */ test_eq_ptr(tor_memstr(haystack, 7, "abc"), haystack + 2); test_eq_ptr(tor_memstr(haystack, 7, "cad"), haystack + 4);