From 74b2bc43fbe61e3a04fe3f5cc9f817be307e13e1 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Tue, 9 Apr 2019 11:59:20 -0400 Subject: [PATCH 1/5] Protect buffers against INT_MAX datalen overflows. Many buffer functions have a hard limit of INT_MAX for datalen, but this limitation is not enforced in all functions: - buf_move_all may exceed that limit with too many chunks - buf_move_to_buf exceeds that limit with invalid buf_flushlen argument - buf_new_with_data may exceed that limit (unit tests only) This patch adds some annotations in some buf_pos_t functions to guarantee that no out of boundary access could occur even if another function lacks safe guards against datalen overflows. [This is a backport of the submitted patch to 0.2.9, where the buf_move_to_buf and buf_new_with_data functions did not exist.] --- src/or/buffers.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/or/buffers.c b/src/or/buffers.c index 89382d1d8e..394ba0ccb8 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -394,6 +394,10 @@ buf_free(buf_t *buf) { if (!buf) return; + if (BUG(buf_out->datalen >= INT_MAX || buf_in->datalen >= INT_MAX)) + return; + if (BUG(buf_out->datalen >= INT_MAX - buf_in->datalen)) + return; buf_clear(buf); buf->magic = 0xdeadbeef; @@ -1034,6 +1038,7 @@ buf_find_pos_of_char(char ch, buf_pos_t *out) static inline int buf_pos_inc(buf_pos_t *pos) { + tor_assert(pos->pos < INT_MAX - 1); ++pos->pos; if (pos->pos == (off_t)pos->chunk->datalen) { if (!pos->chunk->next) @@ -1925,6 +1930,7 @@ buf_find_offset_of_char(buf_t *buf, char ch) { chunk_t *chunk; off_t offset = 0; + tor_assert(buf->datalen < INT_MAX); for (chunk = buf->head; chunk; chunk = chunk->next) { char *cp = memchr(chunk->data, ch, chunk->datalen); if (cp) @@ -2044,6 +2050,7 @@ assert_buf_ok(buf_t *buf) for (ch = buf->head; ch; ch = ch->next) { total += ch->datalen; tor_assert(ch->datalen <= ch->memlen); + tor_assert(ch->datalen < INT_MAX); tor_assert(ch->data >= &ch->mem[0]); tor_assert(ch->data <= &ch->mem[0]+ch->memlen); if (ch->data == &ch->mem[0]+ch->memlen) { From 0fa95308fe5fcce8842530fcae5a49188856e6ac Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Sun, 31 Mar 2019 17:33:11 +0200 Subject: [PATCH 2/5] Check return value of buf_move_to_buf for error. If the concatenation of connection buffer and the buffer of linked connection exceeds INT_MAX bytes, then buf_move_to_buf returns -1 as an error value. This value is currently casted to size_t (variable n_read) and will erroneously lead to an increasement of variable "max_to_read". This in turn can be used to call connection_buf_read_from_socket to store more data inside the buffer than expected and clogging the connection buffer. If the linked connection buffer was able to overflow INT_MAX, the call of buf_move_to_buf would have previously internally triggered an integer overflow, corrupting the state of the connection buffer. Signed-off-by: Tobias Stoeckmann --- src/or/connection.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/or/connection.c b/src/or/connection.c index 791fd95c27..4f636eeb8c 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -3581,6 +3581,10 @@ connection_read_to_buf(connection_t *conn, ssize_t *max_to_read, if (conn->linked_conn) { result = move_buf_to_buf(conn->inbuf, conn->linked_conn->outbuf, &conn->linked_conn->outbuf_flushlen); + if (BUG(result<0)) { + log_warn(LD_BUG, "reading from linked connection buffer failed."); + return -1; + } } else { result = 0; } From c10011532e524846bce300a791f51f298b223f6a Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 9 Apr 2019 12:03:22 -0400 Subject: [PATCH 3/5] Changes file for bug30041 --- changes/bug30041 | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changes/bug30041 diff --git a/changes/bug30041 b/changes/bug30041 new file mode 100644 index 0000000000..801c8f67ac --- /dev/null +++ b/changes/bug30041 @@ -0,0 +1,5 @@ + o Minor bugfixes (hardening): + - Verify in more places that we are not about to create a buffer + with more than INT_MAX bytes, to avoid possible OOB access in the event + of bugs. Fixes bug 30041; bugfix on 0.2.0.16. Found and fixed by + Tobias Stoeckmann. From c35aded00a19f26f3124584e7d0f561cf579efec Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Tue, 12 Mar 2019 20:11:51 +0200 Subject: [PATCH 4/5] Fix #28525 changes file that is breaking CI. --- changes/bug28525 | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/changes/bug28525 b/changes/bug28525 index 392a9265e5..988ffb2192 100644 --- a/changes/bug28525 +++ b/changes/bug28525 @@ -1,8 +1,7 @@ - o Minor bugfixes (address selection): + o Minor features (address selection): - Make Tor aware of the RFC 6598 (Carrier Grade NAT) IP range, which is the subnet 100.64.0.0/10. This is deployed by many ISPs as an alternative to RFC 1918 that does not break existing internal networks. This patch fixes security issues caused by RFC 6518 by blocking control ports on these addresses and warns users if client ports or ExtORPorts are listening on - a RFC 6598 address. Fixes bug 28525; bugfix on 0.4.1.1-alpha. Patch by - Neel Chauhan. + a RFC 6598 address. Closes ticket 28525. Patch by Neel Chauhan. From 37bd7fa50d0901a87084b71299cc8c8786cd1cd8 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 9 Apr 2019 13:14:28 -0400 Subject: [PATCH 5/5] Modify "Protect buffers against INT_MAX datalen overflows." for 0.2.9 --- src/or/buffers.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/or/buffers.c b/src/or/buffers.c index 394ba0ccb8..b36e4ab509 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -394,10 +394,6 @@ buf_free(buf_t *buf) { if (!buf) return; - if (BUG(buf_out->datalen >= INT_MAX || buf_in->datalen >= INT_MAX)) - return; - if (BUG(buf_out->datalen >= INT_MAX - buf_in->datalen)) - return; buf_clear(buf); buf->magic = 0xdeadbeef; @@ -2067,4 +2063,3 @@ assert_buf_ok(buf_t *buf) tor_assert(buf->datalen == total); } } -