sarah
/
tor
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Merge branch 'maint-0.3.2' into release-0.3.2

This commit is contained in:
Nick Mathewson 2018-03-01 16:07:59 -05:00
parent 43d44c04ff d01abb9346
commit bc8b5fb559
6 changed files with 221 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
changes/bug25249 Normal file
View File

@ -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.

3
changes/bug25249.2 Normal file
View File

@ -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.

6
changes/trove-2018-001.1 Normal file
View File

@ -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.

8
changes/trove-2018-004 Normal file
View File

@ -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.

35
src/or/protover.c
View File

@ -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
@ -123,9 +126,14 @@ 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);
low = (uint32_t) tor_parse_ulong(s, 10, 0, MAX_PROTOCOL_VERSION, &ok, &next);
if (!ok)
goto error;
if (next > end_of_range)
@ -138,13 +146,21 @@ parse_version_range(const char *s, const char *end_of_range,
if (*next != '-')
goto error;
s = next+1;
/* ibid */
high = (uint32_t) tor_parse_ulong(s, 10, 0, UINT32_MAX, &ok, &next);
if (!TOR_ISDIGIT(*s)) {
goto error;
}
high = (uint32_t) tor_parse_ulong(s, 10, 0,
MAX_PROTOCOL_VERSION, &ok, &next);
if (!ok)
goto error;
if (next != end_of_range)
goto error;
if (low > high)
goto error;
done:
*high_out = high;
*low_out = low;
@ -195,10 +211,6 @@ parse_single_entry(const char *s, const char *end_of_entry)
goto error;
}
if (range->low > range->high) {
goto error;
}
s = comma;
while (*s == ',' && s < end_of_entry)
++s;
@ -554,6 +566,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 "
@ -618,6 +636,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();

173
src/test/test_protover.c
View File

@ -139,6 +139,70 @@ 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, 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, "");
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, 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, "");
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,8 +242,114 @@ test_protover_all_supported(void *arg)
tt_str_op(msg, OP_EQ, "Link=3-999");
tor_free(msg);
done:
/* 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);
/* 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." */
#ifndef HAVE_RUST
// XXXX let's make this section unconditional: rust should behave the
// XXXX same as C here!
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_();
#endif
done:
tor_end_capture_bugs_();
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 },
{ "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 },
{ "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) \
@ -190,6 +360,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
};