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:12:01 -0400 |
commit | eb966928428a80c105d33bd60bcae5503a1adeb7 (patch) | |
tree | ed5002411cf58d14258c2cab47bceced82eef174 /src/or/protover.c | |
parent | 8340f641c30f8e529b513732be2c931e128227e2 (diff) | |
download | tor-eb966928428a80c105d33bd60bcae5503a1adeb7.tar.gz tor-eb966928428a80c105d33bd60bcae5503a1adeb7.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/or/protover.c')
-rw-r--r-- | src/or/protover.c | 22 |
1 files changed, 22 insertions, 0 deletions
diff --git a/src/or/protover.c b/src/or/protover.c index 45f0377d61..24b3813614 100644 --- a/src/or/protover.c +++ b/src/or/protover.c @@ -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) { |