August 2019 - tor-commits - lists.torproject.org

[tor/maint-0.3.5] Make tor_addr_is_internal_() RFC6598 (Carrier Grade NAT) aware
by teor＠torproject.org 09 Aug '19

09 Aug '19

commit 63b404911441a7691949c475a374569d668a1b32 Author: Neel Chauhan <neel(a)neelc.org> Date: Sat Mar 2 21:25:35 2019 -0500 Make tor_addr_is_internal_() RFC6598 (Carrier Grade NAT) aware Fixes 28525. --- changes/bug28525 | 8 ++++++++ src/common/address.c | 19 +++++++++++++++---- src/test/test_addr.c | 18 ++++++++++++++++++ 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/changes/bug28525 b/changes/bug28525 new file mode 100644 index 000000000..392a9265e --- /dev/null +++ b/changes/bug28525 @@ -0,0 +1,8 @@ + o Minor bugfixes (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. diff --git a/src/common/address.c b/src/common/address.c index 794345a13..71d380538 100644 --- a/src/common/address.c +++ b/src/common/address.c @@ -348,9 +348,18 @@ tor_addr_lookup(const char *name, uint16_t family, tor_addr_t *addr) } } -/** Return true iff ip is an IP reserved to localhost or local networks - * in RFC1918 or RFC4193 or RFC4291. (fec0::/10, deprecated by RFC3879, is - * also treated as internal for now.) +/** Return true iff ip is an IP reserved to localhost or local networks. + * + * If ip is in RFC1918 or RFC4193 or RFC4291, we will return true. + * (fec0::/10, deprecated by RFC3879, is also treated as internal for now + * and will return true.) + * + * If ip is 0.0.0.0 or 100.64.0.0/10 (RFC6598), we will act as: + * - Internal if for_listening is 0, as these addresses are not + * routable on the internet and we won't be publicly accessible to clients. + * - External if for_listening is 1, as clients could connect to us + * from the internet (in the case of 0.0.0.0) or a service provider's + * internal network (in the case of RFC6598). */ int tor_addr_is_internal_(const tor_addr_t *addr, int for_listening, @@ -398,11 +407,13 @@ tor_addr_is_internal_(const tor_addr_t *addr, int for_listening, return 0; } else if (v_family == AF_INET) { - if (for_listening && !iph4) /* special case for binding to 0.0.0.0 */ + /* special case for binding to 0.0.0.0 or 100.64/10 (RFC6598) */ + if (for_listening && (!iph4 || ((iph4 & 0xffc00000) == 0x64400000))) return 0; if (((iph4 & 0xff000000) == 0x0a000000) || /* 10/8 */ ((iph4 & 0xff000000) == 0x00000000) || /* 0/8 */ ((iph4 & 0xff000000) == 0x7f000000) || /* 127/8 */ + ((iph4 & 0xffc00000) == 0x64400000) || /* 100.64/10 */ ((iph4 & 0xffff0000) == 0xa9fe0000) || /* 169.254/16 */ ((iph4 & 0xfff00000) == 0xac100000) || /* 172.16/12 */ ((iph4 & 0xffff0000) == 0xc0a80000)) /* 192.168/16 */ diff --git a/src/test/test_addr.c b/src/test/test_addr.c index be440a092..5c4b6449c 100644 --- a/src/test/test_addr.c +++ b/src/test/test_addr.c @@ -1063,6 +1063,23 @@ test_addr_make_null(void *data) tor_free(zeros); } +#define TEST_ADDR_INTERNAL(a, for_listening, rv) STMT_BEGIN \ + tor_addr_t t; \ + tt_int_op(tor_inet_pton(AF_INET, a, &t.addr.in_addr), OP_EQ, 1); \ + t.family = AF_INET; \ + tt_int_op(tor_addr_is_internal(&t, for_listening), OP_EQ, rv); \ + STMT_END; + +static void +test_addr_rfc6598(void *arg) +{ + (void)arg; + TEST_ADDR_INTERNAL("100.64.0.1", 0, 1); + TEST_ADDR_INTERNAL("100.64.0.1", 1, 0); + done: + ; +} + #define ADDR_LEGACY(name) \ { #name, test_addr_ ## name , 0, NULL, NULL } @@ -1076,6 +1093,7 @@ struct testcase_t addr_tests[] = { { "sockaddr_to_str", test_addr_sockaddr_to_str, 0, NULL, NULL }, { "is_loopback", test_addr_is_loopback, 0, NULL, NULL }, { "make_null", test_addr_make_null, 0, NULL, NULL }, + { "rfc6598", test_addr_rfc6598, 0, NULL, NULL }, END_OF_TESTCASES };

1 0

[tor/maint-0.3.5] Protect buffers against INT_MAX datalen overflows.
by teor＠torproject.org 09 Aug '19

09 Aug '19

commit 74b2bc43fbe61e3a04fe3f5cc9f817be307e13e1 Author: Tobias Stoeckmann <tobias(a)stoeckmann.org> Date: Tue Apr 9 11:59:20 2019 -0400 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 89382d1d8..394ba0ccb 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) {

1 0

[tor/maint-0.3.5] Check return value of buf_move_to_buf for error.
by teor＠torproject.org 09 Aug '19

09 Aug '19

commit a628e36024c4db6e5b178abe3a0b2784c0ab00ec Author: Tobias Stoeckmann <tobias(a)stoeckmann.org> Date: Sun Mar 31 17:33:11 2019 +0200 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 <tobias(a)stoeckmann.org> --- src/or/connection.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/or/connection.c b/src/or/connection.c index 0a2a63509..f18ef7453 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -3561,6 +3561,10 @@ connection_buf_read_from_socket(connection_t *conn, ssize_t *max_to_read, if (conn->linked_conn) { result = buf_move_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; }

1 0

[tor/maint-0.3.5] Protect buffers against INT_MAX datalen overflows.
by teor＠torproject.org 09 Aug '19

09 Aug '19

commit 5a6ab3e7dbf601ae3cc006855f7f4e6c834cbeb2 Author: Tobias Stoeckmann <tobias(a)stoeckmann.org> Date: Sun Mar 31 17:32:41 2019 +0200 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. Signed-off-by: Tobias Stoeckmann <tobias(a)stoeckmann.org> --- src/common/buffers.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/common/buffers.c b/src/common/buffers.c index a01add9be..3951877c5 100644 --- a/src/common/buffers.c +++ b/src/common/buffers.c @@ -273,7 +273,7 @@ buf_t * buf_new_with_data(const char *cp, size_t sz) { /* Validate arguments */ - if (!cp || sz <= 0) { + if (!cp || sz <= 0 || sz >= INT_MAX) { return NULL; } @@ -818,7 +818,7 @@ buf_move_to_buf(buf_t *buf_out, buf_t *buf_in, size_t *buf_flushlen) char b[4096]; size_t cp, len; - if (BUG(buf_out->datalen >= INT_MAX)) + if (BUG(buf_out->datalen >= INT_MAX || *buf_flushlen >= INT_MAX)) return -1; if (BUG(buf_out->datalen >= INT_MAX - *buf_flushlen)) return -1; @@ -850,6 +850,10 @@ buf_move_all(buf_t *buf_out, buf_t *buf_in) tor_assert(buf_out); if (!buf_in) 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; if (buf_out->head == NULL) { buf_out->head = buf_in->head; @@ -917,6 +921,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) @@ -997,6 +1002,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) @@ -1125,6 +1131,7 @@ buf_assert_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) {

1 0

[tor/maint-0.3.5] rephist: fix an undeclared type compilation error
by teor＠torproject.org 09 Aug '19

09 Aug '19

commit 05d25d06b62a9ee2cc77e44a66be2d9a95cae636 Author: teor <teor(a)torproject.org> Date: Tue Apr 16 15:39:45 2019 +1000 rephist: fix an undeclared type compilation error In 0.3.4 and later, we declare write_array as: extern struct bw_array_t *write_array; ... typedef struct bw_array_t bw_array_t; But in 0.2.9, we declare write_array as: typedef struct bw_array_t bw_array_t; extern bw_array_t *write_array; And then again in rephist.c: typedef struct bw_array_t bw_array_t; So some compilers fail with a duplicate declaration error. We backport 684b396ce5, which removes the duplicate declaration. And this commit deals with the undeclared type error. Backports a single line from merge commit 813019cc57. Fixes bug 30184; not in any released version of Tor. --- src/or/rephist.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/or/rephist.h b/src/or/rephist.h index d2f6c66df..303cd74f7 100644 --- a/src/or/rephist.h +++ b/src/or/rephist.h @@ -117,7 +117,7 @@ extern uint32_t rephist_total_num; #ifdef TOR_UNIT_TESTS extern int onion_handshakes_requested[MAX_ONION_HANDSHAKE_TYPE+1]; extern int onion_handshakes_assigned[MAX_ONION_HANDSHAKE_TYPE+1]; -extern bw_array_t *write_array; +extern struct bw_array_t *write_array; #endif #endif

1 0

[tor/maint-0.3.5] Merge branch 'bug30184_029' into bug30184_034
by teor＠torproject.org 09 Aug '19

09 Aug '19

commit dfdd877647ef496b2b4a82e8c1e6040692ecebb5 Merge: 48e990359 031ed59db Author: teor <teor(a)torproject.org> Date: Wed Apr 17 11:51:32 2019 +1000 Merge branch 'bug30184_029' into bug30184_034 "ours" merge, because 0.3.4 has a different set of changes, which already compile.

1 0

[tor/maint-0.3.5] Remove another needless typedef
by teor＠torproject.org 09 Aug '19

09 Aug '19

commit 684b396ce5c0a4d5ea70ec01a22d6d368819c873 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Sep 20 14:34:44 2018 -0400 Remove another needless typedef --- src/or/rephist.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/or/rephist.h b/src/or/rephist.h index c464b34f7..d2f6c66df 100644 --- a/src/or/rephist.h +++ b/src/or/rephist.h @@ -117,9 +117,7 @@ extern uint32_t rephist_total_num; #ifdef TOR_UNIT_TESTS extern int onion_handshakes_requested[MAX_ONION_HANDSHAKE_TYPE+1]; extern int onion_handshakes_assigned[MAX_ONION_HANDSHAKE_TYPE+1]; -typedef struct bw_array_t bw_array_t; extern bw_array_t *write_array; #endif #endif -

1 0

[tor/maint-0.3.5] Changes file for bug30041
by teor＠torproject.org 09 Aug '19

09 Aug '19

commit c24928dd8ffb4c833bae9701921d06072a147938 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Apr 9 12:03:22 2019 -0400 Changes file for bug30041 --- changes/bug30041 | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/changes/bug30041 b/changes/bug30041 new file mode 100644 index 000000000..801c8f67a --- /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.

1 0

[tor/maint-0.3.5] test/relay: add a missing typedef
by teor＠torproject.org 09 Aug '19

09 Aug '19

commit 031ed59dbaf62d9cebd09f98f563c228fe6822f6 Author: teor <teor(a)torproject.org> Date: Wed Apr 17 11:14:05 2019 +1000 test/relay: add a missing typedef In 0.3.4 and later, these functions are declared in rephist.h: STATIC uint64_t find_largest_max(bw_array_t *b); STATIC void commit_max(bw_array_t *b); STATIC void advance_obs(bw_array_t *b); But in 0.2.9, they are declared in rephist.c and test_relay.c. So compilers fail with a "must use 'struct' tag" error. We add the missing struct typedef in test_relay.c, to match the declarations in rephist.c. (Merge commit 813019cc57 moves these functions into rephist.h instead.) Fixes bug 30184; not in any released version of Tor. --- src/test/test_relay.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/test_relay.c b/src/test/test_relay.c index 57dcb2406..d4ebb2338 100644 --- a/src/test/test_relay.c +++ b/src/test/test_relay.c @@ -19,6 +19,8 @@ static or_circuit_t * new_fake_orcirc(channel_t *nchan, channel_t *pchan); static void test_relay_append_cell_to_circuit_queue(void *arg); + +typedef struct bw_array_t bw_array_t; uint64_t find_largest_max(bw_array_t *b); void commit_max(bw_array_t *b); void advance_obs(bw_array_t *b);

1 0

[tor/maint-0.3.5] Tweak comments in tor_vasprintf(), and add a changes file for 30651
by teor＠torproject.org 09 Aug '19

09 Aug '19

commit 0e0cf4abd80249faa23f2bbdb89e62ba96c898f0 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed May 29 09:38:32 2019 -0400 Tweak comments in tor_vasprintf(), and add a changes file for 30651 --- changes/bug30561 | 6 ++++++ src/common/compat.c | 10 ++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/changes/bug30561 b/changes/bug30561 new file mode 100644 index 000000000..afb3f02c6 --- /dev/null +++ b/changes/bug30561 @@ -0,0 +1,6 @@ + o Minor bugfixes (portability): + - Avoid crashing in our tor_vasprintf() implementation on systems that + define neither vasprintf() nor _vscprintf(). (This bug has been here + long enough that we question whether people are running Tor on such + systems, but we're applying the fix out of caution.) Fixes bug 30561; + bugfix on 0.2.8.2-alpha. Found and fixed by Tobias Stoeckmann. diff --git a/src/common/compat.c b/src/common/compat.c index d3bc2f5fe..ee3bf0fd5 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -554,13 +554,16 @@ tor_vasprintf(char **strp, const char *fmt, va_list args) * characters we need. We give it a try on a short buffer first, since * it might be nice to avoid the second vsnprintf call. */ + /* XXXX This code spent a number of years broken (see bug 30651). It is + * possible that no Tor users actually run on systems without vasprintf() or + * _vscprintf(). If so, we should consider removing this code. */ char buf[128]; int len, r; va_list tmp_args; va_copy(tmp_args, args); - /* Use vsnprintf to retrieve needed length. tor_vsnprintf() is not an option - * here because it will simply return -1 if buf is not large enough to hold the - * complete string. + /* Use vsnprintf to retrieve needed length. tor_vsnprintf() is not an + * option here because it will simply return -1 if buf is not large enough + * to hold the complete string. */ len = vsnprintf(buf, sizeof(buf), fmt, tmp_args); va_end(tmp_args); @@ -3550,4 +3553,3 @@ tor_get_avail_disk_space(const char *path) return -1; #endif } -

1 0