From 373a1bc40e88d90e75f4a70b7d2018fe7288035a Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 10 Jan 2011 12:12:11 -0500 Subject: Impose maximum sizes on parsed objects An object, you'll recall, is something between -----BEGIN----- and -----END----- tags in a directory document. Some of our code, as doorss has noted in bug 2352, could assert if one of these ever overflowed SIZE_T_CEILING but not INT_MAX. As a solution, I'm setting a maximum size on a single object such that neither of these limits will ever be hit. I'm also fixing the INT_MAX checks, just to be sure. --- changes/bug2352 | 6 ++++++ src/or/routerparse.c | 11 +++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 changes/bug2352 diff --git a/changes/bug2352 b/changes/bug2352 new file mode 100644 index 000000000..744dbdb20 --- /dev/null +++ b/changes/bug2352 @@ -0,0 +1,6 @@ + o Minor bugfixes + - Fix some potential asserts and partsing issues with grossly + malformed router caches. Fixes bug 2352. Found by doorss. + Bugfix on Tor 0.2.1.27. + + diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 6ca229337..070c61b1a 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -2549,7 +2549,7 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out, goto err; v->good_signature = 1; } else { - if (tok->object_size >= INT_MAX) + if (tok->object_size >= INT_MAX || tok->object_size >= SIZE_T_CEILING) goto err; /* We already parsed a vote from this voter. Use the first one. */ if (v->signature) { @@ -2700,7 +2700,7 @@ networkstatus_parse_detached_signatures(const char *s, const char *eos) voter = tor_malloc_zero(sizeof(networkstatus_voter_info_t)); memcpy(voter->identity_digest, id_digest, DIGEST_LEN); memcpy(voter->signing_key_digest, sk_digest, DIGEST_LEN); - if (tok->object_size >= INT_MAX) + if (tok->object_size >= INT_MAX || tok->object_size >= SIZE_T_CEILING) goto err; voter->signature = tor_memdup(tok->object_body, tok->object_size); voter->signature_len = (int) tok->object_size; @@ -3017,6 +3017,10 @@ static directory_token_t * get_next_token(memarea_t *area, const char **s, const char *eos, token_rule_t *table) { + /** Reject any object at least this big; it is probably an overflow, an + * attack, a bug, or some other nonsense. */ +#define MAX_UNPARSED_OBJECT_SIZE (128*1024) + const char *next, *eol, *obstart; size_t obname_len; int i; @@ -3126,6 +3130,9 @@ get_next_token(memarea_t *area, ebuf[sizeof(ebuf)-1] = '\0'; RET_ERR(ebuf); } + if (next - *s > MAX_UNPARSED_OBJECT_SIZE) + RET_ERR("Couldn't parse object: missing footer or object much too big."); + if (!strcmp(tok->object_type, "RSA PUBLIC KEY")) { /* If it's a public key */ tok->key = crypto_new_pk_env(); if (crypto_pk_read_public_key_from_string(tok->key, obstart, eol-obstart)) -- cgit v1.2.3