mirror of
https://gitlab.torproject.org/tpo/core/tor.git
synced 2024-11-27 22:03:31 +01:00
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
This commit is contained in:
parent
8340f641c3
commit
eb96692842
@ -50,6 +50,11 @@ static const struct {
|
||||
|
||||
#define N_PROTOCOL_NAMES ARRAY_LENGTH(PROTOCOL_NAMES)
|
||||
|
||||
/* Maximum allowed length of any single subprotocol name. */
|
||||
// C_RUST_COUPLED: src/rust/protover/protover.rs
|
||||
// `MAX_PROTOCOL_NAME_LENGTH`
|
||||
static const uint MAX_PROTOCOL_NAME_LENGTH = 100;
|
||||
|
||||
/**
|
||||
* Given a protocol_type_t, return the corresponding string used in
|
||||
* descriptors.
|
||||
@ -195,6 +200,15 @@ parse_single_entry(const char *s, const char *end_of_entry)
|
||||
if (equals == s)
|
||||
goto error;
|
||||
|
||||
/* The name must not be longer than MAX_PROTOCOL_NAME_LENGTH. */
|
||||
if (equals - s > MAX_PROTOCOL_NAME_LENGTH) {
|
||||
log_warn(LD_NET, "When parsing a protocol entry, I got a very large "
|
||||
"protocol name. This is possibly an attack or a bug, unless "
|
||||
"the Tor network truly supports protocol names larger than "
|
||||
"%ud characters. The offending string was: %s",
|
||||
MAX_PROTOCOL_NAME_LENGTH, escaped(out->name));
|
||||
goto error;
|
||||
}
|
||||
out->name = tor_strndup(s, equals-s);
|
||||
|
||||
tor_assert(equals < end_of_entry);
|
||||
@ -397,6 +411,14 @@ expand_protocol_list(const smartlist_t *protos)
|
||||
|
||||
SMARTLIST_FOREACH_BEGIN(protos, const proto_entry_t *, ent) {
|
||||
const char *name = ent->name;
|
||||
if (strlen(name) > MAX_PROTOCOL_NAME_LENGTH) {
|
||||
log_warn(LD_NET, "When expanding a protocol entry, I got a very large "
|
||||
"protocol name. This is possibly an attack or a bug, unless "
|
||||
"the Tor network truly supports protocol names larger than "
|
||||
"%ud characters. The offending string was: %s",
|
||||
MAX_PROTOCOL_NAME_LENGTH, escaped(name));
|
||||
continue;
|
||||
}
|
||||
SMARTLIST_FOREACH_BEGIN(ent->ranges, const proto_range_t *, range) {
|
||||
uint32_t u;
|
||||
for (u = range->low; u <= range->high; ++u) {
|
||||
|
@ -110,6 +110,12 @@ test_protover_parse_fail(void *arg)
|
||||
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);
|
||||
|
||||
done:
|
||||
;
|
||||
}
|
||||
@ -203,6 +209,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);
|
||||
@ -270,6 +285,15 @@ test_protover_all_supported(void *arg)
|
||||
tor_end_capture_bugs_();
|
||||
#endif
|
||||
|
||||
/* 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);
|
||||
|
Loading…
Reference in New Issue
Block a user