aboutsummaryrefslogtreecommitdiff
path: root/src/test/test_protover.c
diff options
context:
space:
mode:
authorIsis Lovecruft <isis@torproject.org>2018-03-29 01:54:05 +0000
committerNick Mathewson <nickm@torproject.org>2018-05-22 12:28:33 -0400
commit056be68b1b5a727bb6cb26d98f37bfa131f76701 (patch)
treec2426a7edf90988bb8e5b8dd07811aeb7461d24d /src/test/test_protover.c
parentadd00045aa56c77b93b1fb4b3cf424588a7baf0e (diff)
downloadtor-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.c24
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);