aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNick Mathewson <nickm@torproject.org>2013-12-11 15:14:43 -0500
committerNick Mathewson <nickm@torproject.org>2014-04-07 22:56:42 -0400
commit9dd115d6b59aaa7d2f444efb9d7992e472fbfb0f (patch)
treea348392d08ba971798ea3dbb6d78f35e9165521b
parent092ac26ea28c17d13bd1d01cab9b3dec276c4dbc (diff)
downloadtor-9dd115d6b59aaa7d2f444efb9d7992e472fbfb0f.tar
tor-9dd115d6b59aaa7d2f444efb9d7992e472fbfb0f.tar.gz
Another 10363 instance: this one in tor_memmem fallback code
-rw-r--r--changes/bug103633
-rw-r--r--src/common/compat.c20
-rw-r--r--src/test/test_util.c4
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);