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

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.
This commit is contained in:
Nick Mathewson 2018-02-15 09:05:55 -05:00
parent 8b405c609e
commit 1fe0bae508
3 changed files with 27 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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.

8
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
@ -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)

21
src/test/test_protover.c
View File

@ -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);
}