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 +…
[View More]+++++
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 "
[View Less]
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.
--…
[View More]-
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);
}
[View Less]