aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNick Mathewson <nickm@torproject.org>2018-02-15 09:05:55 -0500
committerNick Mathewson <nickm@torproject.org>2018-03-01 16:05:17 -0500
commit1fe0bae508120bbf4954de6b590dd0c722a883bc (patch)
treedfbadd8c8d1bb8bdf4b56f509cae9baa93396817
parent8b405c609e82fbfb5470967fc4c45165c708e72b (diff)
downloadtor-1fe0bae508120bbf4954de6b590dd0c722a883bc.tar.gz
tor-1fe0bae508120bbf4954de6b590dd0c722a883bc.zip
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.
-rw-r--r--changes/bug25249.23
-rw-r--r--src/or/protover.c8
-rw-r--r--src/test/test_protover.c21
3 files changed, 27 insertions, 5 deletions
diff --git a/changes/bug25249.2 b/changes/bug25249.2
new file mode 100644
index 0000000000..9058c11071
--- /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 f32316f8e7..a035b5c83b 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 4c41b6db6b..8d061c69ca 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);
}