diff options
author | Nick Mathewson <nickm@torproject.org> | 2013-12-11 15:14:43 -0500 |
---|---|---|
committer | Nick Mathewson <nickm@torproject.org> | 2014-04-07 22:56:42 -0400 |
commit | 9dd115d6b59aaa7d2f444efb9d7992e472fbfb0f (patch) | |
tree | a348392d08ba971798ea3dbb6d78f35e9165521b | |
parent | 092ac26ea28c17d13bd1d01cab9b3dec276c4dbc (diff) | |
download | tor-9dd115d6b59aaa7d2f444efb9d7992e472fbfb0f.tar tor-9dd115d6b59aaa7d2f444efb9d7992e472fbfb0f.tar.gz |
Another 10363 instance: this one in tor_memmem fallback code
-rw-r--r-- | changes/bug10363 | 3 | ||||
-rw-r--r-- | src/common/compat.c | 20 | ||||
-rw-r--r-- | src/test/test_util.c | 4 |
3 files changed, 21 insertions, 6 deletions
diff --git a/changes/bug10363 b/changes/bug10363 index cf5b7fdc8..a3c9905e3 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 d88c5f92d..9b8a0a450 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 65d9d2f87..bf87db70b 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); |