From c8b98ba41ce37662cf14fdb3c6a74ae83b8b0bf8 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 13 Sep 2012 11:45:05 -0400 Subject: [PATCH 1/2] Reject votes (not consensuses) with >64 known-flags Our flag voting code needs to handle unrecognized flags, so it stores them in a 64-bit bitfield. But we never actually checked for too many flags, so we were potentially doing stuff like U64_LITERAL(1)<= 64. That's undefined behavior. Fix for bug 6833; bugfix on 0.2.0.1-alpha. --- changes/bug6833 | 4 ++++ src/or/or.h | 3 +++ src/or/routerparse.c | 5 +++++ 3 files changed, 12 insertions(+) create mode 100644 changes/bug6833 diff --git a/changes/bug6833 b/changes/bug6833 new file mode 100644 index 0000000000..4a6a5d3bbd --- /dev/null +++ b/changes/bug6833 @@ -0,0 +1,4 @@ + o Minor bugfixes (directory authority): + - Reject consensus votes with more than 64 known-flags. We aren't even + close to that limit yet, and our code doesn't handle it + correctly. Fixes bug 6833; bugfix on 0.2.0.1-alpha. diff --git a/src/or/or.h b/src/or/or.h index bb5482bf83..f7914b830d 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -2101,6 +2101,9 @@ typedef struct vote_microdesc_hash_t { typedef struct vote_routerstatus_t { routerstatus_t status; /**< Underlying 'status' object for this router. * Flags are redundant. */ + /** How many known-flags are allowed in a vote? This is the width of + * the flags field of vote_routerstatus_t */ +#define MAX_KNOWN_FLAGS_IN_VOTE 64 uint64_t flags; /**< Bit-field for all recognized flags; index into * networkstatus_t.known_flags. */ char *version; /**< The version that the authority says this router is diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 22f7d78d88..496b90d4ad 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -3004,6 +3004,11 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out, log_warn(LD_DIR, "known-flags not in order"); goto err; } + if (ns->type != NS_TYPE_CONSENSUS && + smartlist_len(ns->known_flags) > MAX_KNOWN_FLAGS_IN_VOTE) { + log_warn(LD_DIR, "Too many known-flags in consensus vote or opinion"); + goto err; + } tok = find_opt_by_keyword(tokens, K_PARAMS); if (tok) { From 68caa834f4ed9cae16a551c9fc63ea982c9f1904 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 14 Sep 2012 10:10:16 -0400 Subject: [PATCH 2/2] document why we only allow 64 flags in votes --- src/or/routerparse.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 496b90d4ad..43a95e88c1 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -3006,6 +3006,11 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out, } if (ns->type != NS_TYPE_CONSENSUS && smartlist_len(ns->known_flags) > MAX_KNOWN_FLAGS_IN_VOTE) { + /* If we allowed more than 64 flags in votes, then parsing them would make + * us invoke undefined behavior whenever we used 1<