commit 00e2310f12dfb91aca2949463b57bd6937f19166 Author: Nick Mathewson nickm@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);