[tor-commits] [tor/master] Don't run off the end of the array-of-freelists

[nickm at torproject.org]

commit 00e2310f12dfb91aca2949463b57bd6937f19166
Author: Nick Mathewson <nickm at torproject.org>
Date:   Wed May 8 12:04:18 2013 -0400

    Don't run off the end of the array-of-freelists
    
    This is a fix for bug 8844, where eugenis correctly notes that there's
    a sentinel value at the end of the list-of-freelists that's never
    actually checked.  It's a bug since the first version of the chunked
    buffer code back in 0.2.0.16-alpha.
    
    This would probably be a crash bug if it ever happens, but nobody's
    ever reported something like this, so I'm unsure whether it can occur.
    It would require write_to_buf, write_to_buf_zlib, read_to_buf, or
    read_to_buf_tls to get an input size of more than 32K.  Still, it's a
    good idea to fix this kind of thing!
---
 changes/bug8844  |    6 ++++++
 src/or/buffers.c |    3 ++-
 src/test/test.c  |   12 ++++++++++++
 3 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/changes/bug8844 b/changes/bug8844
new file mode 100644
index 0000000..320e5f2
--- /dev/null
+++ b/changes/bug8844
@@ -0,0 +1,6 @@
+  o Major bugfixes:
+    - Prevent the get_freelists() function from running off the end of
+      the list of freelists if it somehow gets an unrecognized
+      allocation. Fixes bug 8844; bugfix on 0.2.0.16-alpha. Reported by
+      eugenis.
+
diff --git a/src/or/buffers.c b/src/or/buffers.c
index ad5ab83..9be0476 100644
--- a/src/or/buffers.c
+++ b/src/or/buffers.c
@@ -147,7 +147,8 @@ static INLINE chunk_freelist_t *
 get_freelist(size_t alloc)
 {
   int i;
-  for (i=0; freelists[i].alloc_size <= alloc; ++i) {
+  for (i=0; (freelists[i].alloc_size <= alloc &&
+             freelists[i].alloc_size); ++i ) {
     if (freelists[i].alloc_size == alloc) {
       return &freelists[i];
     }
diff --git a/src/test/test.c b/src/test/test.c
index ddfd633..ae42394 100644
--- a/src/test/test.c
+++ b/src/test/test.c
@@ -802,6 +802,18 @@ test_buffers(void)
   buf_free(buf);
   buf = NULL;
 
+  /* Try adding a string too long for any freelist. */
+  {
+    char *cp = tor_malloc_zero(65536);
+    buf = buf_new();
+    write_to_buf(cp, 65536, buf);
+    tor_free(cp);
+
+    tt_int_op(buf_datalen(buf), ==, 65536);
+    buf_free(buf);
+    buf = NULL;
+  }
+
  done:
   if (buf)
     buf_free(buf);