From f63e06d3dc6757d08ecf26d418ba59bfe060de39 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 13 Feb 2017 11:17:16 -0500 Subject: [PATCH 1/3] Extract the part of tor_version_as_new_as that extracts platform Also add a "strict" mode to reject negative inputs. --- src/or/routerparse.c | 60 +++++++++++++++++++++++++++++++++----------- src/or/routerparse.h | 3 +++ 2 files changed, 49 insertions(+), 14 deletions(-) diff --git a/src/or/routerparse.c b/src/or/routerparse.c index d7e42529cd..28b9c57e4d 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -5517,35 +5517,67 @@ microdescs_parse_from_string(const char *s, const char *eos, * the router is at least as new as the cutoff, else return 0. */ int -tor_version_as_new_as(const char *platform, const char *cutoff) +tor_version_parse_platform(const char *platform, + tor_version_t *router_version, + int strict) { - tor_version_t cutoff_version, router_version; - char *s, *s2, *start; char tmp[128]; + char *s, *s2, *start; - tor_assert(platform); - - if (tor_version_parse(cutoff, &cutoff_version)<0) { - log_warn(LD_BUG,"cutoff version '%s' unparseable.",cutoff); + if (strcmpstart(platform,"Tor ")) /* nonstandard Tor; say 0. */ return 0; - } - if (strcmpstart(platform,"Tor ")) /* nonstandard Tor; be safe and say yes */ - return 1; start = (char *)eat_whitespace(platform+3); - if (!*start) return 0; + if (!*start) return -1; s = (char *)find_whitespace(start); /* also finds '\0', which is fine */ s2 = (char*)eat_whitespace(s); if (!strcmpstart(s2, "(r") || !strcmpstart(s2, "(git-")) s = (char*)find_whitespace(s2); if ((size_t)(s-start+1) >= sizeof(tmp)) /* too big, no */ - return 0; + return -1; strlcpy(tmp, start, s-start+1); - if (tor_version_parse(tmp, &router_version)<0) { + if (tor_version_parse(tmp, router_version)<0) { log_info(LD_DIR,"Router version '%s' unparseable.",tmp); - return 1; /* be safe and say yes */ + return -1; + } + + if (strict) { + if (router_version->major < 0 || + router_version->minor < 0 || + router_version->minor < 0 || + router_version->patchlevel < 0 || + router_version->svn_revision < 0) { + return -1; + } + } + return 1; +} + +/** Parse the Tor version of the platform string platform, + * and compare it to the version in cutoff. Return 1 if + * the router is at least as new as the cutoff, else return 0. + */ +int +tor_version_as_new_as(const char *platform, const char *cutoff) +{ + tor_version_t cutoff_version, router_version; + int r; + tor_assert(platform); + + if (tor_version_parse(cutoff, &cutoff_version)<0) { + log_warn(LD_BUG,"cutoff version '%s' unparseable.",cutoff); + return 0; + } + + r = tor_version_parse_platform(platform, &router_version, 0); + if (r == 0) { + /* nonstandard Tor; be safe and say yes */ + return 1; + } else if (r < 0) { + /* unparseable version; be safe and say yes. */ + return 1; } /* Here's why we don't need to do any special handling for svn revisions: diff --git a/src/or/routerparse.h b/src/or/routerparse.h index 9a3fadca1f..01a5de88e8 100644 --- a/src/or/routerparse.h +++ b/src/or/routerparse.h @@ -45,6 +45,9 @@ MOCK_DECL(addr_policy_t *, router_parse_addr_policy_item_from_string, (const char *s, int assume_action, int *malformed_list)); version_status_t tor_version_is_obsolete(const char *myversion, const char *versionlist); +int tor_version_parse_platform(const char *platform, + tor_version_t *version_out, + int strict); int tor_version_as_new_as(const char *platform, const char *cutoff); int tor_version_parse(const char *s, tor_version_t *out); int tor_version_compare(tor_version_t *a, tor_version_t *b); From 02e05bd74dbec614397b696cfcda6525562a4675 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 13 Feb 2017 11:18:04 -0500 Subject: [PATCH 2/3] When examining descriptors as a dirserver, reject ones with bad versions This is an extra fix for bug 21278: it ensures that these descriptors and platforms will never be listed in a legit consensus. --- changes/bug21278_prevention | 4 ++++ src/or/dirserv.c | 10 ++++++++++ 2 files changed, 14 insertions(+) create mode 100644 changes/bug21278_prevention diff --git a/changes/bug21278_prevention b/changes/bug21278_prevention new file mode 100644 index 0000000000..e07f0a670c --- /dev/null +++ b/changes/bug21278_prevention @@ -0,0 +1,4 @@ + o Minor features (directory authority): + - Directory authorities now reject descriptors that claim to be + malformed versions of Tor. Helps prevent exploitation of bug 21278. + diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 1b614b949e..fa3938b5ec 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -365,6 +365,16 @@ dirserv_get_status_impl(const char *id_digest, const char *nickname, strmap_size(fingerprint_list->fp_by_name), digestmap_size(fingerprint_list->status_by_digest)); + if (platform) { + tor_version_t ver_tmp; + if (tor_version_parse_platform(platform, &ver_tmp, 1) < 0) { + if (msg) { + *msg = "Malformed platform string."; + } + return FP_REJECT; + } + } + /* Versions before Tor 0.2.4.18-rc are too old to support, and are * missing some important security fixes too. Disable them. */ if (platform && !tor_version_as_new_as(platform,"0.2.4.18-rc")) { From 3c4da8a130e41b9096eef360c437cef3cd22a62c Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Tue, 14 Feb 2017 03:52:01 -0500 Subject: [PATCH 3/3] give tor_version_parse_platform some function documentation --- src/or/routerparse.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 28b9c57e4d..c4691b58ce 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -5512,9 +5512,14 @@ microdescs_parse_from_string(const char *s, const char *eos, return result; } -/** Parse the Tor version of the platform string platform, - * and compare it to the version in cutoff. Return 1 if - * the router is at least as new as the cutoff, else return 0. +/** Extract a Tor version from a platform line from a router + * descriptor, and place the result in router_version. + * + * Return 1 on success, -1 on parsing failure, and 0 if the + * platform line does not indicate some version of Tor. + * + * If strict is non-zero, finding any weird version components + * (like negative numbers) counts as a parsing failure. */ int tor_version_parse_platform(const char *platform, @@ -5552,6 +5557,7 @@ tor_version_parse_platform(const char *platform, return -1; } } + return 1; }