March 2018 - tor-commits - lists.torproject.org

[tor/master] Add another NULL-pointer fix for protover.c.
by nickm＠torproject.org 03 Mar '18

03 Mar '18

commit a83650852d3cd00c9916cae74d755ae55a6b506d Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Feb 14 10:45:57 2018 -0500 Add another NULL-pointer fix for protover.c. This one can only be exploited if you can generate a correctly signed consensus, so it's not as bad as 25074. Fixes bug 25251; also tracked as TROVE-2018-004. --- changes/trove-2018-004 | 8 ++++++++ src/or/protover.c | 5 +++++ 2 files changed, 13 insertions(+) diff --git a/changes/trove-2018-004 b/changes/trove-2018-004 new file mode 100644 index 000000000..37e0a89b0 --- /dev/null +++ b/changes/trove-2018-004 @@ -0,0 +1,8 @@ + o Minor bugfixes (denial-of-service): + - Fix a possible crash on malformed consensus. If a consensus had + contained an unparseable protocol line, it could have made clients + and relays crash with a null-pointer exception. To exploit this + issue, however, an attacker would need to be able to subvert the + directory-authority system. Fixes bug 25251; bugfix on + 0.2.9.4-alpha. Also tracked as TROVE-2018-004. + diff --git a/src/or/protover.c b/src/or/protover.c index a75077462..e63036f78 100644 --- a/src/or/protover.c +++ b/src/or/protover.c @@ -624,6 +624,11 @@ protover_all_supported(const char *s, char **missing_out) } smartlist_t *entries = parse_protocol_list(s); + if (BUG(entries == NULL)) { + log_warn(LD_NET, "Received an unparseable protocol list %s" + " from the consensus", escaped(s)); + return 1; + } missing = smartlist_new();

1 0

[tor/maint-0.3.3] Merge branch 'maint-0.3.2' into maint-0.3.3
by nickm＠torproject.org 03 Mar '18

03 Mar '18

commit e0903ebd88a47a03a5e432e1a6c98452ae8a9008 Merge: f7eff2f8c 1ec386561 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Mar 1 16:43:42 2018 -0500 Merge branch 'maint-0.3.2' into maint-0.3.3 "ours" to avoid version bump.

1 0

[tor/master] Forbid UINT32_MAX as a protocol version
by nickm＠torproject.org 03 Mar '18

03 Mar '18

commit 1fe0bae508120bbf4954de6b590dd0c722a883bc Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Feb 15 09:05:55 2018 -0500 Forbid UINT32_MAX as a protocol version The C code and the rust code had different separate integer overflow bugs here. That suggests that we're better off just forbidding this pathological case. Also, add tests for expected behavior on receiving a bad protocol list in a consensus. Fixes another part of 25249. --- changes/bug25249.2 | 3 +++ src/or/protover.c | 8 ++++++-- src/test/test_protover.c | 21 ++++++++++++++++++--- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/changes/bug25249.2 b/changes/bug25249.2 new file mode 100644 index 000000000..9058c1107 --- /dev/null +++ b/changes/bug25249.2 @@ -0,0 +1,3 @@ + o Minor bugfixes (spec conformance): + - Forbid UINT32_MAX as a protocol version. Fixes part of bug 25249; + bugfix on 0.2.9.4-alpha. diff --git a/src/or/protover.c b/src/or/protover.c index f32316f8e..a035b5c83 100644 --- a/src/or/protover.c +++ b/src/or/protover.c @@ -103,6 +103,9 @@ proto_entry_free(proto_entry_t *entry) tor_free(entry); } +/** The largest possible protocol version. */ +#define MAX_PROTOCOL_VERSION (UINT32_MAX-1) + /** * Given a string <b>s</b> and optional end-of-string pointer * <b>end_of_range</b>, parse the protocol range and store it in @@ -130,7 +133,7 @@ parse_version_range(const char *s, const char *end_of_range, /* Note that this wouldn't be safe if we didn't know that eventually, * we'd hit a NUL */ - low = (uint32_t) tor_parse_ulong(s, 10, 0, UINT32_MAX, &ok, &next); + low = (uint32_t) tor_parse_ulong(s, 10, 0, MAX_PROTOCOL_VERSION, &ok, &next); if (!ok) goto error; if (next > end_of_range) @@ -148,7 +151,8 @@ parse_version_range(const char *s, const char *end_of_range, if (!TOR_ISDIGIT(*s)) { goto error; } - high = (uint32_t) tor_parse_ulong(s, 10, 0, UINT32_MAX, &ok, &next); + high = (uint32_t) tor_parse_ulong(s, 10, 0, + MAX_PROTOCOL_VERSION, &ok, &next); if (!ok) goto error; if (next != end_of_range) diff --git a/src/test/test_protover.c b/src/test/test_protover.c index 4c41b6db6..8d061c69c 100644 --- a/src/test/test_protover.c +++ b/src/test/test_protover.c @@ -257,12 +257,27 @@ test_protover_all_supported(void *arg) tt_str_op(msg, OP_EQ, "Sleen=0-2147483648"); tor_free(msg); - /* Rust seems to experience an internal error here */ - tt_assert(! protover_all_supported("Sleen=0-4294967295", &msg)); - tt_str_op(msg, OP_EQ, "Sleen=0-4294967295"); + /* This case is allowed. */ + tt_assert(! protover_all_supported("Sleen=0-4294967294", &msg)); + tt_str_op(msg, OP_EQ, "Sleen=0-4294967294"); tor_free(msg); + /* If we get an unparseable list, we say "yes, that's supported." */ + tor_capture_bugs_(1); + tt_assert(protover_all_supported("Fribble", &msg)); + tt_ptr_op(msg, OP_EQ, NULL); + tor_end_capture_bugs_(); + + /* This case is forbidden. Since it came from a protover_all_supported, + * it can trigger a bug message. */ + tor_capture_bugs_(1); + tt_assert(protover_all_supported("Sleen=0-4294967295", &msg)); + tt_ptr_op(msg, OP_EQ, NULL); + tor_free(msg); + tor_end_capture_bugs_(); + done: + tor_end_capture_bugs_(); tor_free(msg); }

1 0

[tor/master] Add more of Teor's protover tests.
by nickm＠torproject.org 03 Mar '18

03 Mar '18

commit 0953c43c955c4bdb82f0aa86f23f9c0cdcc2c9a1 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Feb 14 11:56:03 2018 -0500 Add more of Teor's protover tests. These are as Teor wrote them; I've disabled the ones that don't pass yet, with XXXX comments. --- src/test/test_protover.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/src/test/test_protover.c b/src/test/test_protover.c index 5e65c019e..609003a83 100644 --- a/src/test/test_protover.c +++ b/src/test/test_protover.c @@ -139,6 +139,80 @@ test_protover_vote(void *arg) tt_str_op(result, OP_EQ, "Bar=3-6,8 Foo=9"); tor_free(result); + /* High threshold */ + result = protover_compute_vote(lst, 3); + tt_str_op(result, OP_EQ, ""); + tor_free(result); + + /* Bad votes: the result must be empty */ + smartlist_clear(lst); + smartlist_add(lst, (void*) "Faux=10-5"); + result = protover_compute_vote(lst, 1); + tt_str_op(result, OP_EQ, ""); + tor_free(result); + + /* This fails in Rust, but not in C */ + smartlist_clear(lst); + smartlist_add(lst, (void*) "Faux=-0"); + result = protover_compute_vote(lst, 1); + tt_str_op(result, OP_EQ, "Faux=0"); + tor_free(result); + + /* Vote large protover lists that are just below the threshold */ + + /* Just below the threshold: Rust */ + smartlist_clear(lst); + smartlist_add(lst, (void*) "Sleen=1-500"); + result = protover_compute_vote(lst, 1); + tt_str_op(result, OP_EQ, "Sleen=1-500"); + tor_free(result); + + /* Just below the threshold: C */ + smartlist_clear(lst); + smartlist_add(lst, (void*) "Sleen=1-65536"); + result = protover_compute_vote(lst, 1); + tt_str_op(result, OP_EQ, "Sleen=1-65536"); + tor_free(result); + + /* Large protover lists that exceed the threshold */ + + /* By adding two votes: Rust */ +#if 0 // XXXXX + smartlist_clear(lst); + smartlist_add(lst, (void*) "Sleen=1-500"); + smartlist_add(lst, (void*) "Sleen=1000"); + result = protover_compute_vote(lst, 1); + tt_str_op(result, OP_EQ, ""); + tor_free(result); +#endif + + /* By adding two votes, C allows us to exceed the limit */ + smartlist_add(lst, (void*) "Sleen=1-65536"); + smartlist_add(lst, (void*) "Sleen=100000"); + result = protover_compute_vote(lst, 1); + tt_str_op(result, OP_EQ, "Sleen=1-65536,100000"); + tor_free(result); + + /* Large integers */ + smartlist_clear(lst); + smartlist_add(lst, (void*) "Sleen=4294967294"); + result = protover_compute_vote(lst, 1); + tt_str_op(result, OP_EQ, "Sleen=4294967294"); + tor_free(result); + + /* This parses, but fails at the vote stage */ + smartlist_clear(lst); + smartlist_add(lst, (void*) "Sleen=4294967295"); + result = protover_compute_vote(lst, 1); + tt_str_op(result, OP_EQ, ""); // XXXX "Sleen=4294967295"); + tor_free(result); + + smartlist_clear(lst); + smartlist_add(lst, (void*) "Sleen=4294967296"); + result = protover_compute_vote(lst, 1); + tt_str_op(result, OP_EQ, ""); + tor_free(result); + done: tor_free(result); smartlist_free(lst); @@ -178,6 +252,16 @@ test_protover_all_supported(void *arg) tt_str_op(msg, OP_EQ, "Link=3-999"); tor_free(msg); + /* CPU/RAM DoS loop: Rust only */ + tt_assert(! protover_all_supported("Sleen=0-2147483648", &msg)); + tt_str_op(msg, OP_EQ, "Sleen=0-2147483648"); + tor_free(msg); + + /* Rust seems to experience an internal error here */ + tt_assert(! protover_all_supported("Sleen=0-4294967295", &msg)); + tt_str_op(msg, OP_EQ, "Sleen=0-4294967295"); + tor_free(msg); + done: tor_free(msg); }

1 0

[tor/master] Add some protover vote round-trip tests from Teor.
by nickm＠torproject.org 03 Mar '18

03 Mar '18

commit d3a1bdbf56c7e41b54fdd1d3169287ca761698a6 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Feb 14 11:47:05 2018 -0500 Add some protover vote round-trip tests from Teor. I've refactored these to be a separate function, to avoid tricky merge conflicts. Some of these are disabled with "XXXX" comments; they should get fixed moving forward. --- src/test/test_protover.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/src/test/test_protover.c b/src/test/test_protover.c index f00955d1b..5e65c019e 100644 --- a/src/test/test_protover.c +++ b/src/test/test_protover.c @@ -182,6 +182,81 @@ test_protover_all_supported(void *arg) tor_free(msg); } +static void +test_protover_vote_roundtrip(void *args) +{ + (void) args; + static const struct { + const char *input; + const char *expected_output; + } examples[] = { + { "Fkrkljdsf", NULL }, + { "Zn=4294967295", NULL }, + { "Zn=4294967295-1", NULL }, + { "Zn=4294967293-4294967295", NULL }, + /* Will fail because of 4294967295. */ + { "Foo=1,3 Bar=3 Baz= Quux=9-12,14,15-16,900 Zn=0,4294967295", + NULL }, + { "Foo=1,3 Bar=3 Baz= Quux=9-12,14,15-16,900 Zn=0,4294967294", + "Bar=3 Foo=1,3 Quux=9-12,14-16,900 Zn=0,4294967294" }, + { "Zu16=0,65536", "Zu16=0,65536" }, + { "N-1=1,2", "N-1=1-2" }, + { "-1=4294967295", NULL }, + { "-1=3", "-1=3" }, + /* junk. */ + { "!!3@*", NULL }, + /* Missing equals sign */ + { "Link=4 Haprauxymatyve Desc=9", NULL }, + { "Link=4 Haprauxymatyve=7 Desc=9", + "Desc=9 Haprauxymatyve=7 Link=4" }, + { "=10-11", NULL }, + { "X=10-11", "X=10-11" }, + { "Link=4 =3 Desc=9", NULL }, + { "Link=4 Z=3 Desc=9", "Desc=9 Link=4 Z=3" }, + { "Link=fred", NULL }, + { "Link=1,fred", NULL }, + { "Link=1,fred,3", NULL }, + { "Link=1,9-8,3", NULL }, + // "These fail at the splitting stage in Rust, but the number parsing + // stage in C." + { "Faux=-1", NULL }, + { "Faux=-1-3", NULL }, + { "Faux=1--1", NULL }, + /* Large integers */ + { "Link=4294967296", NULL }, + /* Large range */ + { "Sleen=1-501", "Sleen=1-501" }, + { "Sleen=1-65537", NULL }, + /* CPU/RAM DoS Loop: Rust only. */ + { "Sleen=0-2147483648", NULL }, + /* Rust seems to experience an internal error here. */ + { "Sleen=0-4294967295", NULL }, + }; + unsigned u; + smartlist_t *votes = smartlist_new(); + char *result = NULL; + + for (u = 0; u < ARRAY_LENGTH(examples); ++u) { + const char *input = examples[u].input; + const char *expected_output = examples[u].expected_output; + + smartlist_add(votes, (void*)input); + result = protover_compute_vote(votes, 1); + if (expected_output != NULL) { + tt_str_op(result, OP_EQ, expected_output); + } else { + tt_str_op(result, OP_EQ, ""); + } + + smartlist_clear(votes); + tor_free(result); + } + + done: + smartlist_free(votes); + tor_free(result); +} + #define PV_TEST(name, flags) \ { #name, test_protover_ ##name, (flags), NULL, NULL } @@ -190,6 +265,7 @@ struct testcase_t protover_tests[] = { PV_TEST(parse_fail, 0), PV_TEST(vote, 0), PV_TEST(all_supported, 0), + PV_TEST(vote_roundtrip, 0), END_OF_TESTCASES };

1 0

[tor/maint-0.3.3] version bump to 0.3.2.10
by nickm＠torproject.org 03 Mar '18

03 Mar '18

commit 1ec386561ef52e781a7b48e8d32250e884a20601 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Mar 1 16:43:35 2018 -0500 version bump to 0.3.2.10 --- configure.ac | 2 +- contrib/win32build/tor-mingw.nsi.in | 2 +- src/win32/orconfig.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index 0fdb84c13..73858d3f2 100644 --- a/configure.ac +++ b/configure.ac @@ -4,7 +4,7 @@ dnl Copyright (c) 2007-2017, The Tor Project, Inc. dnl See LICENSE for licensing information AC_PREREQ([2.63]) -AC_INIT([tor],[0.3.2.9-dev]) +AC_INIT([tor],[0.3.2.10]) AC_CONFIG_SRCDIR([src/or/main.c]) AC_CONFIG_MACRO_DIR([m4]) diff --git a/contrib/win32build/tor-mingw.nsi.in b/contrib/win32build/tor-mingw.nsi.in index 252424619..603f842d6 100644 --- a/contrib/win32build/tor-mingw.nsi.in +++ b/contrib/win32build/tor-mingw.nsi.in @@ -8,7 +8,7 @@ !include "LogicLib.nsh" !include "FileFunc.nsh" !insertmacro GetParameters -!define VERSION "0.3.2.9-dev" +!define VERSION "0.3.2.10" !define INSTALLER "tor-${VERSION}-win32.exe" !define WEBSITE "https://www.torproject.org/" !define LICENSE "LICENSE" diff --git a/src/win32/orconfig.h b/src/win32/orconfig.h index d3645d9fb..83ed63fa4 100644 --- a/src/win32/orconfig.h +++ b/src/win32/orconfig.h @@ -218,7 +218,7 @@ #define USING_TWOS_COMPLEMENT /* Version number of package */ -#define VERSION "0.3.2.9-dev" +#define VERSION "0.3.2.10"

1 0

[tor/maint-0.3.3] Merge branch 'maint-0.3.1' into maint-0.3.2
by nickm＠torproject.org 03 Mar '18

03 Mar '18

commit 5b0ae084008eac13d051026c013862b239212807 Merge: d01abb934 c527a8a9c Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Mar 1 16:43:10 2018 -0500 Merge branch 'maint-0.3.1' into maint-0.3.2 "ours" to avoid version bump.

1 0

[tor/master] Forbid "-0" as a protocol version.
by nickm＠torproject.org 03 Mar '18

03 Mar '18

commit 8b405c609e82fbfb5470967fc4c45165c708e72b Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Feb 15 08:46:13 2018 -0500 Forbid "-0" as a protocol version. Fixes part of 24249; bugfix on 0.2.9.4-alpha. --- changes/bug25249 | 3 +++ src/or/protover.c | 9 +++++++++ src/test/test_protover.c | 6 ++++-- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/changes/bug25249 b/changes/bug25249 new file mode 100644 index 000000000..b4153eeae --- /dev/null +++ b/changes/bug25249 @@ -0,0 +1,3 @@ + o Minor bugfixes (spec conformance): + - Forbid "-0" as a protocol version. Fixes part of bug 25249; bugfix on + 0.2.9.4-alpha. diff --git a/src/or/protover.c b/src/or/protover.c index e63036f78..f32316f8e 100644 --- a/src/or/protover.c +++ b/src/or/protover.c @@ -123,6 +123,11 @@ parse_version_range(const char *s, const char *end_of_range, if (BUG(!end_of_range)) end_of_range = s + strlen(s); // LCOV_EXCL_LINE + /* A range must start with a digit. */ + if (!TOR_ISDIGIT(*s)) { + goto error; + } + /* Note that this wouldn't be safe if we didn't know that eventually, * we'd hit a NUL */ low = (uint32_t) tor_parse_ulong(s, 10, 0, UINT32_MAX, &ok, &next); @@ -138,7 +143,11 @@ parse_version_range(const char *s, const char *end_of_range, if (*next != '-') goto error; s = next+1; + /* ibid */ + if (!TOR_ISDIGIT(*s)) { + goto error; + } high = (uint32_t) tor_parse_ulong(s, 10, 0, UINT32_MAX, &ok, &next); if (!ok) goto error; diff --git a/src/test/test_protover.c b/src/test/test_protover.c index 609003a83..4c41b6db6 100644 --- a/src/test/test_protover.c +++ b/src/test/test_protover.c @@ -151,11 +151,11 @@ test_protover_vote(void *arg) tt_str_op(result, OP_EQ, ""); tor_free(result); - /* This fails in Rust, but not in C */ + /* This fails, since "-0" is not valid. */ smartlist_clear(lst); smartlist_add(lst, (void*) "Faux=-0"); result = protover_compute_vote(lst, 1); - tt_str_op(result, OP_EQ, "Faux=0"); + tt_str_op(result, OP_EQ, ""); tor_free(result); /* Vote large protover lists that are just below the threshold */ @@ -301,6 +301,8 @@ test_protover_vote_roundtrip(void *args) { "Link=1,fred", NULL }, { "Link=1,fred,3", NULL }, { "Link=1,9-8,3", NULL }, + { "Faux=-0", NULL }, + { "Faux=0--0", NULL }, // "These fail at the splitting stage in Rust, but the number parsing // stage in C." { "Faux=-1", NULL },

1 0

[tor/master] Correctly handle NULL returns from parse_protocol_list when voting.
by nickm＠torproject.org 03 Mar '18

03 Mar '18

commit 65f2eec694f18a64291cc85317b9f22dacc1d8e4 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Feb 1 16:33:52 2018 -0500 Correctly handle NULL returns from parse_protocol_list when voting. In some cases we had checked for it, but in others we had not. One of these cases could have been used to remotely cause denial-of-service against directory authorities while they attempted to vote. Fixes TROVE-2018-001. --- changes/trove-2018-001.1 | 6 ++++++ src/or/protover.c | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/changes/trove-2018-001.1 b/changes/trove-2018-001.1 new file mode 100644 index 000000000..f0ee92f40 --- /dev/null +++ b/changes/trove-2018-001.1 @@ -0,0 +1,6 @@ + o Major bugfixes (denial-of-service, directory authority): + - Fix a protocol-list handling bug that could be used to remotely crash + directory authorities with a null-pointer exception. Fixes bug 25074; + bugfix on 0.2.9.4-alpha. Also tracked as TROVE-2018-001. + + diff --git a/src/or/protover.c b/src/or/protover.c index 98957cabd..a75077462 100644 --- a/src/or/protover.c +++ b/src/or/protover.c @@ -554,6 +554,12 @@ protover_compute_vote(const smartlist_t *list_of_proto_strings, // First, parse the inputs and break them into singleton entries. SMARTLIST_FOREACH_BEGIN(list_of_proto_strings, const char *, vote) { smartlist_t *unexpanded = parse_protocol_list(vote); + if (! unexpanded) { + log_warn(LD_NET, "I failed with parsing a protocol list from " + "an authority. The offending string was: %s", + escaped(vote)); + continue; + } smartlist_t *this_vote = expand_protocol_list(unexpanded); if (this_vote == NULL) { log_warn(LD_NET, "When expanding a protocol list from an authority, I "

1 0

[tor/master] Merge branch 'maint-0.2.9' into maint-0.3.1
by nickm＠torproject.org 03 Mar '18

03 Mar '18

commit d4a758e08373854639af6991ec95836fa7c15ca4 Merge: ac1942ac5 c1bb8836f Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Mar 1 16:07:59 2018 -0500 Merge branch 'maint-0.2.9' into maint-0.3.1 changes/bug25249 | 3 + changes/bug25249.2 | 3 + changes/trove-2018-001.1 | 6 ++ changes/trove-2018-004 | 8 +++ src/or/protover.c | 35 ++++++++-- src/test/test_protover.c | 171 +++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 220 insertions(+), 6 deletions(-)

1 0