diff options
author | Isis Lovecruft <isis@torproject.org> | 2018-03-29 01:54:05 +0000 |
---|---|---|
committer | Nick Mathewson <nickm@torproject.org> | 2018-05-22 12:28:33 -0400 |
commit | 056be68b1b5a727bb6cb26d98f37bfa131f76701 (patch) | |
tree | c2426a7edf90988bb8e5b8dd07811aeb7461d24d /src/test/test_protover.c | |
parent | add00045aa56c77b93b1fb4b3cf424588a7baf0e (diff) | |
download | tor-056be68b1b5a727bb6cb26d98f37bfa131f76701.tar.gz tor-056be68b1b5a727bb6cb26d98f37bfa131f76701.zip |
protover: TROVE-2018-005 Fix potential DoS in protover protocol parsing.
In protover.c, the `expand_protocol_list()` function expands a `smartlist_t` of
`proto_entry_t`s to their protocol name concatenated with each version number.
For example, given a `proto_entry_t` like so:
proto_entry_t *proto = tor_malloc(sizeof(proto_entry_t));
proto_range_t *range = tor_malloc_zero(sizeof(proto_range_t));
proto->name = tor_strdup("DoSaaaaaaaaaaaaaaaaaaaaaa[19KB]aaa");
proto->ranges = smartlist_new();
range->low = 1;
range->high = 65536;
smartlist_add(proto->ranges, range);
(Where `[19KB]` is roughly 19KB of `"a"` bytes.) This would expand in
`expand_protocol_list()` to a `smartlist_t` containing 65536 copies of the
string, e.g.:
"DoSaaaaaaaaaaaaaaaaaaaaaa[19KB]aaa=1"
"DoSaaaaaaaaaaaaaaaaaaaaaa[19KB]aaa=2"
[…]
"DoSaaaaaaaaaaaaaaaaaaaaaa[19KB]aaa=65535"
Thus constituting a potential resource exhaustion attack.
The Rust implementation is not subject to this attack, because it instead
expands the above string into a `HashMap<String, HashSet<u32>` prior to #24031,
and a `HashMap<UnvalidatedProtocol, ProtoSet>` after). Neither Rust version is
subject to this attack, because it only stores the `String` once per protocol.
(Although a related, but apparently of too minor impact to be usable, DoS bug
has been fixed in #24031. [0])
[0]: https://bugs.torproject.org/24031
* ADDS hard limit on protocol name lengths in protover.c and checks in
parse_single_entry() and expand_protocol_list().
* ADDS tests to ensure the bug is caught.
* FIXES #25517: https://bugs.torproject.org/25517
Diffstat (limited to 'src/test/test_protover.c')
-rw-r--r-- | src/test/test_protover.c | 24 |
1 files changed, 24 insertions, 0 deletions
diff --git a/src/test/test_protover.c b/src/test/test_protover.c index 7bf1471ebd..7a4fffad8b 100644 --- a/src/test/test_protover.c +++ b/src/test/test_protover.c @@ -125,6 +125,13 @@ test_protover_parse_fail(void *arg) /* Broken range */ elts = parse_protocol_list("Link=1,9-8,3"); tt_ptr_op(elts, OP_EQ, NULL); + + /* Protocol name too long */ + elts = parse_protocol_list("DoSaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"); + tt_ptr_op(elts, OP_EQ, NULL); + #endif done: ; @@ -219,6 +226,15 @@ test_protover_vote(void *arg) tt_str_op(result, OP_EQ, ""); tor_free(result); + /* Protocol name too long */ + smartlist_clear(lst); + smartlist_add(lst, (void*) "DoSaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"); + result = protover_compute_vote(lst, 1); + tt_str_op(result, OP_EQ, ""); + tor_free(result); + done: tor_free(result); smartlist_free(lst); @@ -300,6 +316,14 @@ test_protover_all_supported(void *arg) tt_assert(protover_all_supported("Sleen=0-4294967295", &msg)); tor_end_capture_bugs_(); + /* Protocol name too long */ + tor_capture_bugs_(1); + tt_assert(protover_all_supported("DoSaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaa=1-65536", &msg)); + tor_end_capture_bugs_(); + done: tor_end_capture_bugs_(); tor_free(msg); |