Skip to content

Commit 3cea86e

Browse files
committed
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).
1 parent 12a7298 commit 3cea86e

File tree

2 files changed

+43
-8
lines changed

2 files changed

+43
-8
lines changed

Diff for: changes/buf-sentinel

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
o Major features (security fixes):
2+
3+
- Prevent a class of security bugs caused by treating the contents
4+
of a buffer chunk as if they were a NUL-terminated string. At
5+
least one such bug seems to be present in all currently used
6+
versions of Tor, and would allow an attacker to remotely crash
7+
most Tor instances, especially those compiled with extra compiler
8+
hardening. With this defense in place, such bugs can't crash Tor,
9+
though we should still fix them as they occur. Closes ticket 20384
10+
(TROVE-2016-10-001).
11+

Diff for: src/or/buffers.c

+32-8
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,33 @@ static int parse_socks_client(const uint8_t *data, size_t datalen,
6969

7070
#define CHUNK_HEADER_LEN STRUCT_OFFSET(chunk_t, mem[0])
7171

72+
/* We leave this many NUL bytes at the end of the buffer. */
73+
#define SENTINEL_LEN 4
74+
75+
/* Header size plus NUL bytes at the end */
76+
#define CHUNK_OVERHEAD (CHUNK_HEADER_LEN + SENTINEL_LEN)
77+
7278
/** Return the number of bytes needed to allocate a chunk to hold
7379
* <b>memlen</b> bytes. */
74-
#define CHUNK_ALLOC_SIZE(memlen) (CHUNK_HEADER_LEN + (memlen))
80+
#define CHUNK_ALLOC_SIZE(memlen) (CHUNK_OVERHEAD + (memlen))
7581
/** Return the number of usable bytes in a chunk allocated with
7682
* malloc(<b>memlen</b>). */
77-
#define CHUNK_SIZE_WITH_ALLOC(memlen) ((memlen) - CHUNK_HEADER_LEN)
83+
#define CHUNK_SIZE_WITH_ALLOC(memlen) ((memlen) - CHUNK_OVERHEAD)
84+
85+
#define DEBUG_SENTINEL
86+
87+
#ifdef DEBUG_SENTINEL
88+
#define DBG_S(s) s
89+
#else
90+
#define DBG_S(s) (void)0
91+
#endif
92+
93+
#define CHUNK_SET_SENTINEL(chunk, alloclen) do { \
94+
uint8_t *a = (uint8_t*) &(chunk)->mem[(chunk)->memlen]; \
95+
DBG_S(uint8_t *b = &((uint8_t*)(chunk))[(alloclen)-SENTINEL_LEN]); \
96+
DBG_S(tor_assert(a == b)); \
97+
memset(a,0,SENTINEL_LEN); \
98+
} while (0)
7899

79100
/** Return the next character in <b>chunk</b> onto which data can be appended.
80101
* 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)
131152
ch->memlen = CHUNK_SIZE_WITH_ALLOC(alloc);
132153
total_bytes_allocated_in_chunks += alloc;
133154
ch->data = &ch->mem[0];
155+
CHUNK_SET_SENTINEL(ch, alloc);
134156
return ch;
135157
}
136158

@@ -140,18 +162,20 @@ static INLINE chunk_t *
140162
chunk_grow(chunk_t *chunk, size_t sz)
141163
{
142164
off_t offset;
143-
size_t memlen_orig = chunk->memlen;
165+
const size_t memlen_orig = chunk->memlen;
166+
const size_t orig_alloc = CHUNK_ALLOC_SIZE(memlen_orig);
167+
const size_t new_alloc = CHUNK_ALLOC_SIZE(sz);
144168
tor_assert(sz > chunk->memlen);
145169
offset = chunk->data - chunk->mem;
146-
chunk = tor_realloc(chunk, CHUNK_ALLOC_SIZE(sz));
170+
chunk = tor_realloc(chunk, new_alloc);
147171
chunk->memlen = sz;
148172
chunk->data = chunk->mem + offset;
149173
#ifdef DEBUG_CHUNK_ALLOC
150-
tor_assert(chunk->DBG_alloc == CHUNK_ALLOC_SIZE(memlen_orig));
151-
chunk->DBG_alloc = CHUNK_ALLOC_SIZE(sz);
174+
tor_assert(chunk->DBG_alloc == orig_alloc);
175+
chunk->DBG_alloc = new_alloc;
152176
#endif
153-
total_bytes_allocated_in_chunks +=
154-
CHUNK_ALLOC_SIZE(sz) - CHUNK_ALLOC_SIZE(memlen_orig);
177+
total_bytes_allocated_in_chunks += new_alloc - orig_alloc;
178+
CHUNK_SET_SENTINEL(chunk, new_alloc);
155179
return chunk;
156180
}
157181

0 commit comments

Comments
 (0)