December 2016 - tor-commits - lists.torproject.org

[tor/maint-0.2.6] Merge branch 'maint-0.2.4' into maint-0.2.5
by nickm＠torproject.org 21 Dec '16

21 Dec '16

commit 7d3d56e59a3df16615881007a77b77e30fecb831 Merge: db58d4d 8f857c2 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Dec 20 18:19:41 2016 -0500 Merge branch 'maint-0.2.4' into maint-0.2.5 (ours merge -- there is a separate 0.2.5 patch for 20384.)

1 0

[tor/maint-0.2.5] Add a one-word sentinel value of 0x0 at the end of each buf_t chunk
by nickm＠torproject.org 21 Dec '16

21 Dec '16

commit 39ef34352317d66d6fe33a0632f1db057de89f48 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Oct 14 09:38:12 2016 -0400 Add a one-word sentinel value of 0x0 at the end of each buf_t chunk This helps protect against bugs where any part of a buf_t's memory is passed to a function that expects a NUL-terminated input. --- src/or/buffers.c | 41 +++++++++++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/src/or/buffers.c b/src/or/buffers.c index ae73c70..a60c7c0 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -66,12 +66,33 @@ static int parse_socks_client(const uint8_t *data, size_t datalen, #define CHUNK_HEADER_LEN STRUCT_OFFSET(chunk_t, mem[0]) +/* We leave this many NUL bytes at the end of the buffer. */ +#define SENTINEL_LEN 4 + +/* Header size plus NUL bytes at the end */ +#define CHUNK_OVERHEAD (CHUNK_HEADER_LEN + SENTINEL_LEN) + /** Return the number of bytes needed to allocate a chunk to hold * memlen bytes. */ -#define CHUNK_ALLOC_SIZE(memlen) (CHUNK_HEADER_LEN + (memlen)) +#define CHUNK_ALLOC_SIZE(memlen) (CHUNK_OVERHEAD + (memlen)) /** Return the number of usable bytes in a chunk allocated with * malloc(memlen). */ -#define CHUNK_SIZE_WITH_ALLOC(memlen) ((memlen) - CHUNK_HEADER_LEN) +#define CHUNK_SIZE_WITH_ALLOC(memlen) ((memlen) - CHUNK_OVERHEAD) + +#define DEBUG_SENTINEL + +#ifdef DEBUG_SENTINEL +#define DBG_S(s) s +#else +#define DBG_S(s) (void)0 +#endif + +#define CHUNK_SET_SENTINEL(chunk, alloclen) do { \ + uint8_t *a = (uint8_t*) &(chunk)->mem[(chunk)->memlen]; \ + DBG_S(uint8_t *b = &((uint8_t*)(chunk))[(alloclen)-SENTINEL_LEN]); \ + DBG_S(tor_assert(a == b)); \ + memset(a,0,SENTINEL_LEN); \ + } while (0) /** Return the next character in chunk onto which data can be appended. * If the chunk is full, this might be off the end of chunk->mem. */ @@ -207,6 +228,7 @@ chunk_new_with_alloc_size(size_t alloc) ch->datalen = 0; ch->memlen = CHUNK_SIZE_WITH_ALLOC(alloc); ch->data = &ch->mem[0]; + CHUNK_SET_SENTINEL(ch, alloc); return ch; } #else @@ -236,6 +258,7 @@ chunk_new_with_alloc_size(size_t alloc) ch->memlen = CHUNK_SIZE_WITH_ALLOC(alloc); total_bytes_allocated_in_chunks += alloc; ch->data = &ch->mem[0]; + CHUNK_SET_SENTINEL(ch, alloc); return ch; } #endif @@ -246,18 +269,20 @@ static INLINE chunk_t * chunk_grow(chunk_t *chunk, size_t sz) { off_t offset; - size_t memlen_orig = chunk->memlen; + const size_t memlen_orig = chunk->memlen; + const size_t orig_alloc = CHUNK_ALLOC_SIZE(memlen_orig); + const size_t new_alloc = CHUNK_ALLOC_SIZE(sz); tor_assert(sz > chunk->memlen); offset = chunk->data - chunk->mem; - chunk = tor_realloc(chunk, CHUNK_ALLOC_SIZE(sz)); + chunk = tor_realloc(chunk, new_alloc); chunk->memlen = sz; chunk->data = chunk->mem + offset; #ifdef DEBUG_CHUNK_ALLOC - tor_assert(chunk->DBG_alloc == CHUNK_ALLOC_SIZE(memlen_orig)); - chunk->DBG_alloc = CHUNK_ALLOC_SIZE(sz); + tor_assert(chunk->DBG_alloc == orig_alloc); + chunk->DBG_alloc = new_alloc; #endif - total_bytes_allocated_in_chunks += - CHUNK_ALLOC_SIZE(sz) - CHUNK_ALLOC_SIZE(memlen_orig); + total_bytes_allocated_in_chunks += new_alloc - orig_alloc; + CHUNK_SET_SENTINEL(chunk, new_alloc); return chunk; }

1 0

[tor/maint-0.2.4] Add a one-word sentinel value of 0x0 at the end of each buf_t chunk
by nickm＠torproject.org 21 Dec '16

21 Dec '16

commit 8f857c23b755cda41dbb8adf6b14d5defb76a00c Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Oct 14 09:38:12 2016 -0400 Add a one-word sentinel value of 0x0 at the end of each buf_t chunk This helps protect against bugs where any part of a buf_t's memory is passed to a function that expects a NUL-terminated input. --- src/or/buffers.c | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/src/or/buffers.c b/src/or/buffers.c index ab3346d..e2e59eb 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -75,12 +75,33 @@ typedef struct chunk_t { #define CHUNK_HEADER_LEN STRUCT_OFFSET(chunk_t, mem[0]) +/* We leave this many NUL bytes at the end of the buffer. */ +#define SENTINEL_LEN 4 + +/* Header size plus NUL bytes at the end */ +#define CHUNK_OVERHEAD (CHUNK_HEADER_LEN + SENTINEL_LEN) + /** Return the number of bytes needed to allocate a chunk to hold * memlen bytes. */ -#define CHUNK_ALLOC_SIZE(memlen) (CHUNK_HEADER_LEN + (memlen)) +#define CHUNK_ALLOC_SIZE(memlen) (CHUNK_OVERHEAD + (memlen)) /** Return the number of usable bytes in a chunk allocated with * malloc(memlen). */ -#define CHUNK_SIZE_WITH_ALLOC(memlen) ((memlen) - CHUNK_HEADER_LEN) +#define CHUNK_SIZE_WITH_ALLOC(memlen) ((memlen) - CHUNK_OVERHEAD) + +#define DEBUG_SENTINEL + +#ifdef DEBUG_SENTINEL +#define DBG_S(s) s +#else +#define DBG_S(s) (void)0 +#endif + +#define CHUNK_SET_SENTINEL(chunk, alloclen) do { \ + uint8_t *a = (uint8_t*) &(chunk)->mem[(chunk)->memlen]; \ + DBG_S(uint8_t *b = &((uint8_t*)(chunk))[(alloclen)-SENTINEL_LEN]); \ + DBG_S(tor_assert(a == b)); \ + memset(a,0,SENTINEL_LEN); \ + } while (0) /** Return the next character in chunk onto which data can be appended. * If the chunk is full, this might be off the end of chunk->mem. */ @@ -204,6 +225,7 @@ chunk_new_with_alloc_size(size_t alloc) ch->datalen = 0; ch->memlen = CHUNK_SIZE_WITH_ALLOC(alloc); ch->data = &ch->mem[0]; + CHUNK_SET_SENTINEL(ch, alloc); return ch; } #else @@ -221,6 +243,7 @@ chunk_new_with_alloc_size(size_t alloc) ch->datalen = 0; ch->memlen = CHUNK_SIZE_WITH_ALLOC(alloc); ch->data = &ch->mem[0]; + CHUNK_SET_SENTINEL(ch, alloc); return ch; } #endif @@ -231,11 +254,13 @@ static INLINE chunk_t * chunk_grow(chunk_t *chunk, size_t sz) { off_t offset; + const size_t new_alloc = CHUNK_ALLOC_SIZE(sz); tor_assert(sz > chunk->memlen); offset = chunk->data - chunk->mem; - chunk = tor_realloc(chunk, CHUNK_ALLOC_SIZE(sz)); + chunk = tor_realloc(chunk, new_alloc); chunk->memlen = sz; chunk->data = chunk->mem + offset; + CHUNK_SET_SENTINEL(chunk, new_alloc); return chunk; }

1 0

[tor/maint-0.2.7] Add a one-word sentinel value of 0x0 at the end of each buf_t chunk
by nickm＠torproject.org 21 Dec '16

21 Dec '16

commit 39ef34352317d66d6fe33a0632f1db057de89f48 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Oct 14 09:38:12 2016 -0400 Add a one-word sentinel value of 0x0 at the end of each buf_t chunk This helps protect against bugs where any part of a buf_t's memory is passed to a function that expects a NUL-terminated input. --- src/or/buffers.c | 41 +++++++++++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/src/or/buffers.c b/src/or/buffers.c index ae73c70..a60c7c0 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -66,12 +66,33 @@ static int parse_socks_client(const uint8_t *data, size_t datalen, #define CHUNK_HEADER_LEN STRUCT_OFFSET(chunk_t, mem[0]) +/* We leave this many NUL bytes at the end of the buffer. */ +#define SENTINEL_LEN 4 + +/* Header size plus NUL bytes at the end */ +#define CHUNK_OVERHEAD (CHUNK_HEADER_LEN + SENTINEL_LEN) + /** Return the number of bytes needed to allocate a chunk to hold * memlen bytes. */ -#define CHUNK_ALLOC_SIZE(memlen) (CHUNK_HEADER_LEN + (memlen)) +#define CHUNK_ALLOC_SIZE(memlen) (CHUNK_OVERHEAD + (memlen)) /** Return the number of usable bytes in a chunk allocated with * malloc(memlen). */ -#define CHUNK_SIZE_WITH_ALLOC(memlen) ((memlen) - CHUNK_HEADER_LEN) +#define CHUNK_SIZE_WITH_ALLOC(memlen) ((memlen) - CHUNK_OVERHEAD) + +#define DEBUG_SENTINEL + +#ifdef DEBUG_SENTINEL +#define DBG_S(s) s +#else +#define DBG_S(s) (void)0 +#endif + +#define CHUNK_SET_SENTINEL(chunk, alloclen) do { \ + uint8_t *a = (uint8_t*) &(chunk)->mem[(chunk)->memlen]; \ + DBG_S(uint8_t *b = &((uint8_t*)(chunk))[(alloclen)-SENTINEL_LEN]); \ + DBG_S(tor_assert(a == b)); \ + memset(a,0,SENTINEL_LEN); \ + } while (0) /** Return the next character in chunk onto which data can be appended. * If the chunk is full, this might be off the end of chunk->mem. */ @@ -207,6 +228,7 @@ chunk_new_with_alloc_size(size_t alloc) ch->datalen = 0; ch->memlen = CHUNK_SIZE_WITH_ALLOC(alloc); ch->data = &ch->mem[0]; + CHUNK_SET_SENTINEL(ch, alloc); return ch; } #else @@ -236,6 +258,7 @@ chunk_new_with_alloc_size(size_t alloc) ch->memlen = CHUNK_SIZE_WITH_ALLOC(alloc); total_bytes_allocated_in_chunks += alloc; ch->data = &ch->mem[0]; + CHUNK_SET_SENTINEL(ch, alloc); return ch; } #endif @@ -246,18 +269,20 @@ static INLINE chunk_t * chunk_grow(chunk_t *chunk, size_t sz) { off_t offset; - size_t memlen_orig = chunk->memlen; + const size_t memlen_orig = chunk->memlen; + const size_t orig_alloc = CHUNK_ALLOC_SIZE(memlen_orig); + const size_t new_alloc = CHUNK_ALLOC_SIZE(sz); tor_assert(sz > chunk->memlen); offset = chunk->data - chunk->mem; - chunk = tor_realloc(chunk, CHUNK_ALLOC_SIZE(sz)); + chunk = tor_realloc(chunk, new_alloc); chunk->memlen = sz; chunk->data = chunk->mem + offset; #ifdef DEBUG_CHUNK_ALLOC - tor_assert(chunk->DBG_alloc == CHUNK_ALLOC_SIZE(memlen_orig)); - chunk->DBG_alloc = CHUNK_ALLOC_SIZE(sz); + tor_assert(chunk->DBG_alloc == orig_alloc); + chunk->DBG_alloc = new_alloc; #endif - total_bytes_allocated_in_chunks += - CHUNK_ALLOC_SIZE(sz) - CHUNK_ALLOC_SIZE(memlen_orig); + total_bytes_allocated_in_chunks += new_alloc - orig_alloc; + CHUNK_SET_SENTINEL(chunk, new_alloc); return chunk; }

1 0

[tor/maint-0.2.6] Add a one-word sentinel value of 0x0 at the end of each buf_t chunk
by nickm＠torproject.org 21 Dec '16

21 Dec '16

commit b6227edae1d8318b694029800a26e17a2a960af5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Oct 14 09:38:12 2016 -0400 Add a one-word sentinel value of 0x0 at the end of each buf_t chunk This helps protect against bugs where any part of a buf_t's memory is passed to a function that expects a NUL-terminated input. It also closes TROVE-2016-10-001 (aka bug 20384). --- changes/buf-sentinel | 11 +++++++++++ src/or/buffers.c | 40 ++++++++++++++++++++++++++++++++-------- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/changes/buf-sentinel b/changes/buf-sentinel new file mode 100644 index 0000000..7c5b829 --- /dev/null +++ b/changes/buf-sentinel @@ -0,0 +1,11 @@ + o Major features (security fixes): + + - Prevent a class of security bugs caused by treating the contents + of a buffer chunk as if they were a NUL-terminated string. At + least one such bug seems to be present in all currently used + versions of Tor, and would allow an attacker to remotely crash + most Tor instances, especially those compiled with extra compiler + hardening. With this defense in place, such bugs can't crash Tor, + though we should still fix them as they occur. Closes ticket 20384 + (TROVE-2016-10-001). + diff --git a/src/or/buffers.c b/src/or/buffers.c index be99744..74aebcc 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -69,12 +69,33 @@ static int parse_socks_client(const uint8_t *data, size_t datalen, #define CHUNK_HEADER_LEN STRUCT_OFFSET(chunk_t, mem[0]) +/* We leave this many NUL bytes at the end of the buffer. */ +#define SENTINEL_LEN 4 + +/* Header size plus NUL bytes at the end */ +#define CHUNK_OVERHEAD (CHUNK_HEADER_LEN + SENTINEL_LEN) + /** Return the number of bytes needed to allocate a chunk to hold * memlen bytes. */ -#define CHUNK_ALLOC_SIZE(memlen) (CHUNK_HEADER_LEN + (memlen)) +#define CHUNK_ALLOC_SIZE(memlen) (CHUNK_OVERHEAD + (memlen)) /** Return the number of usable bytes in a chunk allocated with * malloc(memlen). */ -#define CHUNK_SIZE_WITH_ALLOC(memlen) ((memlen) - CHUNK_HEADER_LEN) +#define CHUNK_SIZE_WITH_ALLOC(memlen) ((memlen) - CHUNK_OVERHEAD) + +#define DEBUG_SENTINEL + +#ifdef DEBUG_SENTINEL +#define DBG_S(s) s +#else +#define DBG_S(s) (void)0 +#endif + +#define CHUNK_SET_SENTINEL(chunk, alloclen) do { \ + uint8_t *a = (uint8_t*) &(chunk)->mem[(chunk)->memlen]; \ + DBG_S(uint8_t *b = &((uint8_t*)(chunk))[(alloclen)-SENTINEL_LEN]); \ + DBG_S(tor_assert(a == b)); \ + memset(a,0,SENTINEL_LEN); \ + } while (0) /** Return the next character in chunk onto which data can be appended. * If the chunk is full, this might be off the end of chunk->mem. */ @@ -131,6 +152,7 @@ chunk_new_with_alloc_size(size_t alloc) ch->memlen = CHUNK_SIZE_WITH_ALLOC(alloc); total_bytes_allocated_in_chunks += alloc; ch->data = &ch->mem[0]; + CHUNK_SET_SENTINEL(ch, alloc); return ch; } @@ -140,18 +162,20 @@ static INLINE chunk_t * chunk_grow(chunk_t *chunk, size_t sz) { off_t offset; - size_t memlen_orig = chunk->memlen; + const size_t memlen_orig = chunk->memlen; + const size_t orig_alloc = CHUNK_ALLOC_SIZE(memlen_orig); + const size_t new_alloc = CHUNK_ALLOC_SIZE(sz); tor_assert(sz > chunk->memlen); offset = chunk->data - chunk->mem; - chunk = tor_realloc(chunk, CHUNK_ALLOC_SIZE(sz)); + chunk = tor_realloc(chunk, new_alloc); chunk->memlen = sz; chunk->data = chunk->mem + offset; #ifdef DEBUG_CHUNK_ALLOC - tor_assert(chunk->DBG_alloc == CHUNK_ALLOC_SIZE(memlen_orig)); - chunk->DBG_alloc = CHUNK_ALLOC_SIZE(sz); + tor_assert(chunk->DBG_alloc == orig_alloc); + chunk->DBG_alloc = new_alloc; #endif - total_bytes_allocated_in_chunks += - CHUNK_ALLOC_SIZE(sz) - CHUNK_ALLOC_SIZE(memlen_orig); + total_bytes_allocated_in_chunks += new_alloc - orig_alloc; + CHUNK_SET_SENTINEL(chunk, new_alloc); return chunk; }

1 0

[tor/maint-0.2.7] Merge branch 'maint-0.2.6' into maint-0.2.7
by nickm＠torproject.org 21 Dec '16

21 Dec '16

commit 2673b4b7a87bbdc880dd235f490dfb7b8f3c64c9 Merge: a9c8a5f b6227ed Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Dec 20 18:23:19 2016 -0500 Merge branch 'maint-0.2.6' into maint-0.2.7 changes/buf-sentinel | 11 +++++++++++ src/or/buffers.c | 40 ++++++++++++++++++++++++++++++++-------- 2 files changed, 43 insertions(+), 8 deletions(-)

1 0

[tor/maint-0.2.5] Merge branch 'maint-0.2.4' into maint-0.2.5
by nickm＠torproject.org 21 Dec '16

21 Dec '16

commit 7d3d56e59a3df16615881007a77b77e30fecb831 Merge: db58d4d 8f857c2 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Dec 20 18:19:41 2016 -0500 Merge branch 'maint-0.2.4' into maint-0.2.5 (ours merge -- there is a separate 0.2.5 patch for 20384.)

1 0

[tor/maint-0.2.5] Add a one-word sentinel value of 0x0 at the end of each buf_t chunk
by nickm＠torproject.org 21 Dec '16

21 Dec '16

commit 8f857c23b755cda41dbb8adf6b14d5defb76a00c Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Oct 14 09:38:12 2016 -0400 Add a one-word sentinel value of 0x0 at the end of each buf_t chunk This helps protect against bugs where any part of a buf_t's memory is passed to a function that expects a NUL-terminated input. --- src/or/buffers.c | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/src/or/buffers.c b/src/or/buffers.c index ab3346d..e2e59eb 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -75,12 +75,33 @@ typedef struct chunk_t { #define CHUNK_HEADER_LEN STRUCT_OFFSET(chunk_t, mem[0]) +/* We leave this many NUL bytes at the end of the buffer. */ +#define SENTINEL_LEN 4 + +/* Header size plus NUL bytes at the end */ +#define CHUNK_OVERHEAD (CHUNK_HEADER_LEN + SENTINEL_LEN) + /** Return the number of bytes needed to allocate a chunk to hold * memlen bytes. */ -#define CHUNK_ALLOC_SIZE(memlen) (CHUNK_HEADER_LEN + (memlen)) +#define CHUNK_ALLOC_SIZE(memlen) (CHUNK_OVERHEAD + (memlen)) /** Return the number of usable bytes in a chunk allocated with * malloc(memlen). */ -#define CHUNK_SIZE_WITH_ALLOC(memlen) ((memlen) - CHUNK_HEADER_LEN) +#define CHUNK_SIZE_WITH_ALLOC(memlen) ((memlen) - CHUNK_OVERHEAD) + +#define DEBUG_SENTINEL + +#ifdef DEBUG_SENTINEL +#define DBG_S(s) s +#else +#define DBG_S(s) (void)0 +#endif + +#define CHUNK_SET_SENTINEL(chunk, alloclen) do { \ + uint8_t *a = (uint8_t*) &(chunk)->mem[(chunk)->memlen]; \ + DBG_S(uint8_t *b = &((uint8_t*)(chunk))[(alloclen)-SENTINEL_LEN]); \ + DBG_S(tor_assert(a == b)); \ + memset(a,0,SENTINEL_LEN); \ + } while (0) /** Return the next character in chunk onto which data can be appended. * If the chunk is full, this might be off the end of chunk->mem. */ @@ -204,6 +225,7 @@ chunk_new_with_alloc_size(size_t alloc) ch->datalen = 0; ch->memlen = CHUNK_SIZE_WITH_ALLOC(alloc); ch->data = &ch->mem[0]; + CHUNK_SET_SENTINEL(ch, alloc); return ch; } #else @@ -221,6 +243,7 @@ chunk_new_with_alloc_size(size_t alloc) ch->datalen = 0; ch->memlen = CHUNK_SIZE_WITH_ALLOC(alloc); ch->data = &ch->mem[0]; + CHUNK_SET_SENTINEL(ch, alloc); return ch; } #endif @@ -231,11 +254,13 @@ static INLINE chunk_t * chunk_grow(chunk_t *chunk, size_t sz) { off_t offset; + const size_t new_alloc = CHUNK_ALLOC_SIZE(sz); tor_assert(sz > chunk->memlen); offset = chunk->data - chunk->mem; - chunk = tor_realloc(chunk, CHUNK_ALLOC_SIZE(sz)); + chunk = tor_realloc(chunk, new_alloc); chunk->memlen = sz; chunk->data = chunk->mem + offset; + CHUNK_SET_SENTINEL(chunk, new_alloc); return chunk; }

1 0

[tor/maint-0.2.7] Add a one-word sentinel value of 0x0 at the end of each buf_t chunk
by nickm＠torproject.org 21 Dec '16

21 Dec '16

commit b6227edae1d8318b694029800a26e17a2a960af5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Oct 14 09:38:12 2016 -0400 Add a one-word sentinel value of 0x0 at the end of each buf_t chunk This helps protect against bugs where any part of a buf_t's memory is passed to a function that expects a NUL-terminated input. It also closes TROVE-2016-10-001 (aka bug 20384). --- changes/buf-sentinel | 11 +++++++++++ src/or/buffers.c | 40 ++++++++++++++++++++++++++++++++-------- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/changes/buf-sentinel b/changes/buf-sentinel new file mode 100644 index 0000000..7c5b829 --- /dev/null +++ b/changes/buf-sentinel @@ -0,0 +1,11 @@ + o Major features (security fixes): + + - Prevent a class of security bugs caused by treating the contents + of a buffer chunk as if they were a NUL-terminated string. At + least one such bug seems to be present in all currently used + versions of Tor, and would allow an attacker to remotely crash + most Tor instances, especially those compiled with extra compiler + hardening. With this defense in place, such bugs can't crash Tor, + though we should still fix them as they occur. Closes ticket 20384 + (TROVE-2016-10-001). + diff --git a/src/or/buffers.c b/src/or/buffers.c index be99744..74aebcc 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -69,12 +69,33 @@ static int parse_socks_client(const uint8_t *data, size_t datalen, #define CHUNK_HEADER_LEN STRUCT_OFFSET(chunk_t, mem[0]) +/* We leave this many NUL bytes at the end of the buffer. */ +#define SENTINEL_LEN 4 + +/* Header size plus NUL bytes at the end */ +#define CHUNK_OVERHEAD (CHUNK_HEADER_LEN + SENTINEL_LEN) + /** Return the number of bytes needed to allocate a chunk to hold * memlen bytes. */ -#define CHUNK_ALLOC_SIZE(memlen) (CHUNK_HEADER_LEN + (memlen)) +#define CHUNK_ALLOC_SIZE(memlen) (CHUNK_OVERHEAD + (memlen)) /** Return the number of usable bytes in a chunk allocated with * malloc(memlen). */ -#define CHUNK_SIZE_WITH_ALLOC(memlen) ((memlen) - CHUNK_HEADER_LEN) +#define CHUNK_SIZE_WITH_ALLOC(memlen) ((memlen) - CHUNK_OVERHEAD) + +#define DEBUG_SENTINEL + +#ifdef DEBUG_SENTINEL +#define DBG_S(s) s +#else +#define DBG_S(s) (void)0 +#endif + +#define CHUNK_SET_SENTINEL(chunk, alloclen) do { \ + uint8_t *a = (uint8_t*) &(chunk)->mem[(chunk)->memlen]; \ + DBG_S(uint8_t *b = &((uint8_t*)(chunk))[(alloclen)-SENTINEL_LEN]); \ + DBG_S(tor_assert(a == b)); \ + memset(a,0,SENTINEL_LEN); \ + } while (0) /** Return the next character in chunk onto which data can be appended. * If the chunk is full, this might be off the end of chunk->mem. */ @@ -131,6 +152,7 @@ chunk_new_with_alloc_size(size_t alloc) ch->memlen = CHUNK_SIZE_WITH_ALLOC(alloc); total_bytes_allocated_in_chunks += alloc; ch->data = &ch->mem[0]; + CHUNK_SET_SENTINEL(ch, alloc); return ch; } @@ -140,18 +162,20 @@ static INLINE chunk_t * chunk_grow(chunk_t *chunk, size_t sz) { off_t offset; - size_t memlen_orig = chunk->memlen; + const size_t memlen_orig = chunk->memlen; + const size_t orig_alloc = CHUNK_ALLOC_SIZE(memlen_orig); + const size_t new_alloc = CHUNK_ALLOC_SIZE(sz); tor_assert(sz > chunk->memlen); offset = chunk->data - chunk->mem; - chunk = tor_realloc(chunk, CHUNK_ALLOC_SIZE(sz)); + chunk = tor_realloc(chunk, new_alloc); chunk->memlen = sz; chunk->data = chunk->mem + offset; #ifdef DEBUG_CHUNK_ALLOC - tor_assert(chunk->DBG_alloc == CHUNK_ALLOC_SIZE(memlen_orig)); - chunk->DBG_alloc = CHUNK_ALLOC_SIZE(sz); + tor_assert(chunk->DBG_alloc == orig_alloc); + chunk->DBG_alloc = new_alloc; #endif - total_bytes_allocated_in_chunks += - CHUNK_ALLOC_SIZE(sz) - CHUNK_ALLOC_SIZE(memlen_orig); + total_bytes_allocated_in_chunks += new_alloc - orig_alloc; + CHUNK_SET_SENTINEL(chunk, new_alloc); return chunk; }

1 0

[tor/maint-0.2.8] Add a one-word sentinel value of 0x0 at the end of each buf_t chunk
by nickm＠torproject.org 21 Dec '16

21 Dec '16

commit 8f857c23b755cda41dbb8adf6b14d5defb76a00c Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Oct 14 09:38:12 2016 -0400 Add a one-word sentinel value of 0x0 at the end of each buf_t chunk This helps protect against bugs where any part of a buf_t's memory is passed to a function that expects a NUL-terminated input. --- src/or/buffers.c | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/src/or/buffers.c b/src/or/buffers.c index ab3346d..e2e59eb 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -75,12 +75,33 @@ typedef struct chunk_t { #define CHUNK_HEADER_LEN STRUCT_OFFSET(chunk_t, mem[0]) +/* We leave this many NUL bytes at the end of the buffer. */ +#define SENTINEL_LEN 4 + +/* Header size plus NUL bytes at the end */ +#define CHUNK_OVERHEAD (CHUNK_HEADER_LEN + SENTINEL_LEN) + /** Return the number of bytes needed to allocate a chunk to hold * memlen bytes. */ -#define CHUNK_ALLOC_SIZE(memlen) (CHUNK_HEADER_LEN + (memlen)) +#define CHUNK_ALLOC_SIZE(memlen) (CHUNK_OVERHEAD + (memlen)) /** Return the number of usable bytes in a chunk allocated with * malloc(memlen). */ -#define CHUNK_SIZE_WITH_ALLOC(memlen) ((memlen) - CHUNK_HEADER_LEN) +#define CHUNK_SIZE_WITH_ALLOC(memlen) ((memlen) - CHUNK_OVERHEAD) + +#define DEBUG_SENTINEL + +#ifdef DEBUG_SENTINEL +#define DBG_S(s) s +#else +#define DBG_S(s) (void)0 +#endif + +#define CHUNK_SET_SENTINEL(chunk, alloclen) do { \ + uint8_t *a = (uint8_t*) &(chunk)->mem[(chunk)->memlen]; \ + DBG_S(uint8_t *b = &((uint8_t*)(chunk))[(alloclen)-SENTINEL_LEN]); \ + DBG_S(tor_assert(a == b)); \ + memset(a,0,SENTINEL_LEN); \ + } while (0) /** Return the next character in chunk onto which data can be appended. * If the chunk is full, this might be off the end of chunk->mem. */ @@ -204,6 +225,7 @@ chunk_new_with_alloc_size(size_t alloc) ch->datalen = 0; ch->memlen = CHUNK_SIZE_WITH_ALLOC(alloc); ch->data = &ch->mem[0]; + CHUNK_SET_SENTINEL(ch, alloc); return ch; } #else @@ -221,6 +243,7 @@ chunk_new_with_alloc_size(size_t alloc) ch->datalen = 0; ch->memlen = CHUNK_SIZE_WITH_ALLOC(alloc); ch->data = &ch->mem[0]; + CHUNK_SET_SENTINEL(ch, alloc); return ch; } #endif @@ -231,11 +254,13 @@ static INLINE chunk_t * chunk_grow(chunk_t *chunk, size_t sz) { off_t offset; + const size_t new_alloc = CHUNK_ALLOC_SIZE(sz); tor_assert(sz > chunk->memlen); offset = chunk->data - chunk->mem; - chunk = tor_realloc(chunk, CHUNK_ALLOC_SIZE(sz)); + chunk = tor_realloc(chunk, new_alloc); chunk->memlen = sz; chunk->data = chunk->mem + offset; + CHUNK_SET_SENTINEL(chunk, new_alloc); return chunk; }

1 0