- tor-commits - lists.torproject.org

[tor/release-0.2.2] Handle out-of-range values in tor_parse_* integer functions
by arma＠torproject.org 10 May '12

10 May '12

commit 9b344628ed8f15543dc7780cc2a5cdd1b8f656cf Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon May 7 12:25:59 2012 -0400 Handle out-of-range values in tor_parse_* integer functions The underlying strtoX functions handle overflow by saturating and setting errno to ERANGE. If the min/max arguments to the tor_parse_* functions are equal to the minimum/maximum of the underlying type, then with the old approach, we wouldn't treat a too-large value as genuinely broken. Found this while looking at bug 5786; bugfix on 19da1f36 (in Tor 0.0.9), which introduced these functions. --- changes/bug5786_range | 8 ++++++++ src/common/util.c | 7 +++++++ src/test/test_util.c | 15 +++++++++++++++ 3 files changed, 30 insertions(+), 0 deletions(-) diff --git a/changes/bug5786_range b/changes/bug5786_range new file mode 100644 index 0000000..40ac4d2 --- /dev/null +++ b/changes/bug5786_range @@ -0,0 +1,8 @@ + o Minor bugfixes: + - Make our number-parsing functions always treat too-large values + as an error, even when those values exceed the width of the + underlying type. Previously, if the caller provided these + functions with minima or maxima set to the extreme values of the + underlying integer type, these functions would return those + values on overflow rather than treating overflow as an error. + Fix for part of bug 5786; bugfix on Tor 0.0.9. \ No newline at end of file diff --git a/src/common/util.c b/src/common/util.c index e3cd154..7d2fc4d 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -764,6 +764,9 @@ tor_digest256_is_zero(const char *digest) /* Helper: common code to check whether the result of a strtol or strtoul or * strtoll is correct. */ #define CHECK_STRTOX_RESULT() \ + /* Did an overflow occur? */ \ + if (errno == ERANGE) \ + goto err; \ /* Was at least one character converted? */ \ if (endptr == s) \ goto err; \ @@ -800,6 +803,7 @@ tor_parse_long(const char *s, int base, long min, long max, char *endptr; long r; + errno = 0; r = strtol(s, &endptr, base); CHECK_STRTOX_RESULT(); } @@ -812,6 +816,7 @@ tor_parse_ulong(const char *s, int base, unsigned long min, char *endptr; unsigned long r; + errno = 0; r = strtoul(s, &endptr, base); CHECK_STRTOX_RESULT(); } @@ -823,6 +828,7 @@ tor_parse_double(const char *s, double min, double max, int *ok, char **next) char *endptr; double r; + errno = 0; r = strtod(s, &endptr); CHECK_STRTOX_RESULT(); } @@ -836,6 +842,7 @@ tor_parse_uint64(const char *s, int base, uint64_t min, char *endptr; uint64_t r; + errno = 0; #ifdef HAVE_STRTOULL r = (uint64_t)strtoull(s, &endptr, base); #elif defined(MS_WINDOWS) diff --git a/src/test/test_util.c b/src/test/test_util.c index 23cd059..ee745c5 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -283,6 +283,21 @@ test_util_strmisc(void) test_assert(i == 1); } + { + /* Test tor_parse_* where we overflow/underflow the underlying type. */ + /* This string should overflow 64-bit ints. */ +#define TOOBIG "100000000000000000000000000" + test_eq(0L, tor_parse_long(TOOBIG, 10, LONG_MIN, LONG_MAX, &i, NULL)); + test_eq(i, 0); + test_eq(0L, tor_parse_long("-"TOOBIG, 10, LONG_MIN, LONG_MAX, &i, NULL)); + test_eq(i, 0); + test_eq(0UL, tor_parse_ulong(TOOBIG, 10, 0, ULONG_MAX, &i, NULL)); + test_eq(i, 0); + test_eq(U64_LITERAL(0), tor_parse_uint64(TOOBIG, 10, + 0, UINT64_MAX, &i, NULL)); + test_eq(i, 0); + } + /* Test failing snprintf cases */ test_eq(-1, tor_snprintf(buf, 0, "Foo")); test_eq(-1, tor_snprintf(buf, 2, "Foo"));

1 0

[tor/release-0.2.2] Merge branch 'bug5786_range_022' into maint-0.2.2
by arma＠torproject.org 10 May '12

10 May '12

commit 79c4c8195a9da15dc13866dd4a706807c79dba46 Merge: 5bbf04d 9b34462 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu May 10 15:38:57 2012 -0400 Merge branch 'bug5786_range_022' into maint-0.2.2 changes/bug5786_range | 8 ++++++++ src/common/util.c | 7 +++++++ src/test/test_util.c | 15 +++++++++++++++ 3 files changed, 30 insertions(+), 0 deletions(-)

1 0

[tor/release-0.2.2] Add changes/bug5760
by arma＠torproject.org 10 May '12

10 May '12

commit 5bbf04dc9727de393ab1c42440124d875c79c13e Author: Ravi Chandra Padmala <neenaoffline(a)gmail.com> Date: Thu May 10 12:53:16 2012 +0530 Add changes/bug5760 --- changes/bug5760 | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/changes/bug5760 b/changes/bug5760 new file mode 100644 index 0000000..a26407b --- /dev/null +++ b/changes/bug5760 @@ -0,0 +1,3 @@ + o Major bugfixes: + - End AUTHCHALLENGE error response messages with a CRLF. Fixes bug 5760; + bugfix on 0.2.3.16-alpha, and backported to maint-0.2.2

1 0

[tor/master] Only disable cert chaining on the first TLS handshake
by nickm＠torproject.org 10 May '12

10 May '12

commit f0212197cccf461e431d6807a94ea0fdc411e179 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Apr 27 12:13:56 2012 -0400 Only disable cert chaining on the first TLS handshake If the client uses a v2 cipherlist on the renegotiation handshake, it looks as if they could fail to get a good cert chain from the server, since they server would re-disable certificate chaining. This patch makes it so the code that make the server side of the first v2 handshake special can get called only once. Fix for 4591; bugfix on 0.2.0.20-rc. --- changes/bug4591 | 6 ++++++ src/common/tortls.c | 4 +++- 2 files changed, 9 insertions(+), 1 deletions(-) diff --git a/changes/bug4591 b/changes/bug4591 new file mode 100644 index 0000000..59b25a5 --- /dev/null +++ b/changes/bug4591 @@ -0,0 +1,6 @@ + o Minor bugfixes: + - If the client fails to set a reasonable set of ciphersuites + during its v2 handshake renegotiation, allow the renegotiation + to continue nevertheless (i.e., send all the required + certificates). Fix for bug 4591; bugfix on 0.2.0.20-rc. + diff --git a/src/common/tortls.c b/src/common/tortls.c index 4c9d218..abdd411 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -965,7 +965,9 @@ tor_tls_server_info_callback(const SSL *ssl, int type, int val) /* Now check the cipher list. */ if (tor_tls_client_is_using_v2_ciphers(ssl, ADDR(tls))) { - /*XXXX_TLS keep this from happening more than once! */ + if (tls->wasV2Handshake) + return; /* We already turned this stuff off for the first handshake; + * This is a renegotiation. */ /* Yes, we're casting away the const from ssl. This is very naughty of us. * Let's hope openssl doesn't notice! */

1 0

[tor/master] Merge remote-tracking branch 'public/bug4591'
by nickm＠torproject.org 10 May '12

10 May '12

commit 62f8e3926d62ff4aaefedfe89355f04f3a8d74fa Merge: 0b1a334 f021219 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu May 10 15:55:12 2012 -0400 Merge remote-tracking branch 'public/bug4591' changes/bug4591 | 6 ++++++ src/common/tortls.c | 4 +++- 2 files changed, 9 insertions(+), 1 deletions(-)

1 0

[tor/master] Check more thoroughly for dups when parsing networkstatus parameters
by nickm＠torproject.org 10 May '12

10 May '12

commit c8a0cceae279eb2b27c51a358e00942a9967ca77 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon May 7 12:38:28 2012 -0400 Check more thoroughly for dups when parsing networkstatus parameters See changes file for details. Partial fix for bug 5786; fix on 0.2.2.2-alpha. --- changes/bug5786_nodups | 7 +++++++ src/or/dirvote.c | 4 ++-- src/or/routerparse.c | 17 +++++++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/changes/bug5786_nodups b/changes/bug5786_nodups new file mode 100644 index 0000000..795b668 --- /dev/null +++ b/changes/bug5786_nodups @@ -0,0 +1,7 @@ + o Major bugfixes (directory authority): + - Check more thoroughly to prevent a rogue authority from + double-voting on any consensus directory parameter. Previously, + authorities would crash in this case if the total number of votes + for any parameter exceeded the number of active voters, but would + let it pass otherwise. Partial fix for bug 5786; bugfix on + 0.2.2.2-alpha. diff --git a/src/or/dirvote.c b/src/or/dirvote.c index cddb658..4848917 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -668,10 +668,10 @@ dirvote_compute_params(smartlist_t *votes, int method, int total_authorities) const char *next_param; int ok=0; eq = strchr(param, '='); - tor_assert(i<n_votes); + tor_assert(i<n_votes); /* Make sure we prevented vote-stuffing. */ vals[i++] = (int32_t) tor_parse_long(eq+1, 10, INT32_MIN, INT32_MAX, &ok, NULL); - tor_assert(ok); + tor_assert(ok); /* Already checked these when parsing. */ if (param_sl_idx+1 == smartlist_len(param_list)) next_param = NULL; diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 7cc8b96..c5ce174 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -2821,6 +2821,7 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out, int i, inorder, n_signatures = 0; memarea_t *area = NULL, *rs_area = NULL; consensus_flavor_t flav = FLAV_NS; + char *last_kwd=NULL; tor_assert(s); @@ -2977,15 +2978,18 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out, tok = find_opt_by_keyword(tokens, K_PARAMS); if (tok) { + int any_dups = 0; inorder = 1; ns->net_params = smartlist_new(); for (i = 0; i < tok->n_args; ++i) { int ok=0; char *eq = strchr(tok->args[i], '='); + size_t eq_pos; if (!eq) { log_warn(LD_DIR, "Bad element '%s' in params", escaped(tok->args[i])); goto err; } + eq_pos = eq-tok->args[i]; tor_parse_long(eq+1, 10, INT32_MIN, INT32_MAX, &ok, NULL); if (!ok) { log_warn(LD_DIR, "Bad element '%s' in params", escaped(tok->args[i])); @@ -2995,12 +2999,24 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out, log_warn(LD_DIR, "%s >= %s", tok->args[i-1], tok->args[i]); inorder = 0; } + if (last_kwd && eq_pos == strlen(last_kwd) && + fast_memeq(last_kwd, tok->args[i], eq_pos)) { + log_warn(LD_DIR, "Duplicate value for %s parameter", + escaped(tok->args[i])); + any_dups = 1; + } + tor_free(last_kwd); + last_kwd = tor_strndup(tok->args[i], eq_pos); smartlist_add(ns->net_params, tor_strdup(tok->args[i])); } if (!inorder) { log_warn(LD_DIR, "params not in order"); goto err; } + if (any_dups) { + log_warn(LD_DIR, "Duplicate in parameters"); + goto err; + } } ns->voters = smartlist_new(); @@ -3339,6 +3355,7 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out, } if (rs_area) memarea_drop_all(rs_area); + tor_free(last_kwd); return ns; }

1 0

[tor/master] Detect out-of-bounds bwweightscale values early in the voting process
by nickm＠torproject.org 10 May '12

10 May '12

commit d9ba9f91d25210f10f740d243899dbe99f8acb60 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon May 7 12:44:34 2012 -0400 Detect out-of-bounds bwweightscale values early in the voting process If the authorities agreed on a sufficiently bad bwweightscale value (<=0 or == INT32_MAX), the bandwidth algorithm could make the voters assert while computing the consensus. Fix for bug5786; bugfix on 0.2.2.17-alpha --- changes/bug5786_nocrash | 7 +++++++ src/or/dirvote.c | 6 +++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/changes/bug5786_nocrash b/changes/bug5786_nocrash new file mode 100644 index 0000000..ec6c5d8 --- /dev/null +++ b/changes/bug5786_nocrash @@ -0,0 +1,7 @@ + o Major bugfixes (directory authorties): + - When computing weight parameters, behave more robustly in the + presence of a bad bwweightscale value. Previously, the + authorities would crash if they agreed on a sufficiently browken + weight_scale value: now, they use a reasonable default and carry + on. Partial fix for 5786; bugfix on 0.2.2.17-alpha. + diff --git a/src/or/dirvote.c b/src/or/dirvote.c index 4848917..20dc8c2 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -1005,7 +1005,7 @@ networkstatus_compute_bw_weights_v10(smartlist_t *chunks, int64_t G, /* We cast down the weights to 32 bit ints on the assumption that * weight_scale is ~= 10000. We need to ensure a rogue authority * doesn't break this assumption to rig our weights */ - tor_assert(0 < weight_scale && weight_scale < INT32_MAX); + tor_assert(0 < weight_scale && weight_scale <= INT32_MAX); /* * Provide Wgm=Wgg, Wmm=1, Wem=Wee, Weg=Wed. May later determine @@ -1233,7 +1233,7 @@ networkstatus_compute_bw_weights_v9(smartlist_t *chunks, int64_t G, int64_t M, /* We cast down the weights to 32 bit ints on the assumption that * weight_scale is ~= 10000. We need to ensure a rogue authority * doesn't break this assumption to rig our weights */ - tor_assert(0 < weight_scale && weight_scale < INT32_MAX); + tor_assert(0 < weight_scale && weight_scale <= INT32_MAX); if (Wgg < 0 || Wgg > weight_scale) { log_warn(LD_DIR, "Bw %s: Wgg="I64_FORMAT"! G="I64_FORMAT @@ -2019,7 +2019,7 @@ networkstatus_compute_consensus(smartlist_t *votes, int ok=0; char *eq = strchr(bw_weight_param, '='); if (eq) { - weight_scale = tor_parse_long(eq+1, 10, INT32_MIN, INT32_MAX, &ok, + weight_scale = tor_parse_long(eq+1, 10, 1, INT32_MAX, &ok, NULL); if (!ok) { log_warn(LD_DIR, "Bad element '%s' in bw weight param",

1 0

[tor/master] Merge branch 'bug5786'
by nickm＠torproject.org 10 May '12

10 May '12

commit 0b1a334842f632b935d0b23757722d06bd145724 Merge: c78a426 d9ba9f9 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu May 10 15:44:41 2012 -0400 Merge branch 'bug5786' changes/bug5786_nocrash | 7 +++++++ changes/bug5786_nodups | 7 +++++++ src/or/dirvote.c | 10 +++++----- src/or/routerparse.c | 17 +++++++++++++++++ 4 files changed, 36 insertions(+), 5 deletions(-)

1 0

[tor/master] Handle out-of-range values in tor_parse_* integer functions
by nickm＠torproject.org 10 May '12

10 May '12

commit 9b344628ed8f15543dc7780cc2a5cdd1b8f656cf Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon May 7 12:25:59 2012 -0400 Handle out-of-range values in tor_parse_* integer functions The underlying strtoX functions handle overflow by saturating and setting errno to ERANGE. If the min/max arguments to the tor_parse_* functions are equal to the minimum/maximum of the underlying type, then with the old approach, we wouldn't treat a too-large value as genuinely broken. Found this while looking at bug 5786; bugfix on 19da1f36 (in Tor 0.0.9), which introduced these functions. --- changes/bug5786_range | 8 ++++++++ src/common/util.c | 7 +++++++ src/test/test_util.c | 15 +++++++++++++++ 3 files changed, 30 insertions(+), 0 deletions(-) diff --git a/changes/bug5786_range b/changes/bug5786_range new file mode 100644 index 0000000..40ac4d2 --- /dev/null +++ b/changes/bug5786_range @@ -0,0 +1,8 @@ + o Minor bugfixes: + - Make our number-parsing functions always treat too-large values + as an error, even when those values exceed the width of the + underlying type. Previously, if the caller provided these + functions with minima or maxima set to the extreme values of the + underlying integer type, these functions would return those + values on overflow rather than treating overflow as an error. + Fix for part of bug 5786; bugfix on Tor 0.0.9. \ No newline at end of file diff --git a/src/common/util.c b/src/common/util.c index e3cd154..7d2fc4d 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -764,6 +764,9 @@ tor_digest256_is_zero(const char *digest) /* Helper: common code to check whether the result of a strtol or strtoul or * strtoll is correct. */ #define CHECK_STRTOX_RESULT() \ + /* Did an overflow occur? */ \ + if (errno == ERANGE) \ + goto err; \ /* Was at least one character converted? */ \ if (endptr == s) \ goto err; \ @@ -800,6 +803,7 @@ tor_parse_long(const char *s, int base, long min, long max, char *endptr; long r; + errno = 0; r = strtol(s, &endptr, base); CHECK_STRTOX_RESULT(); } @@ -812,6 +816,7 @@ tor_parse_ulong(const char *s, int base, unsigned long min, char *endptr; unsigned long r; + errno = 0; r = strtoul(s, &endptr, base); CHECK_STRTOX_RESULT(); } @@ -823,6 +828,7 @@ tor_parse_double(const char *s, double min, double max, int *ok, char **next) char *endptr; double r; + errno = 0; r = strtod(s, &endptr); CHECK_STRTOX_RESULT(); } @@ -836,6 +842,7 @@ tor_parse_uint64(const char *s, int base, uint64_t min, char *endptr; uint64_t r; + errno = 0; #ifdef HAVE_STRTOULL r = (uint64_t)strtoull(s, &endptr, base); #elif defined(MS_WINDOWS) diff --git a/src/test/test_util.c b/src/test/test_util.c index 23cd059..ee745c5 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -283,6 +283,21 @@ test_util_strmisc(void) test_assert(i == 1); } + { + /* Test tor_parse_* where we overflow/underflow the underlying type. */ + /* This string should overflow 64-bit ints. */ +#define TOOBIG "100000000000000000000000000" + test_eq(0L, tor_parse_long(TOOBIG, 10, LONG_MIN, LONG_MAX, &i, NULL)); + test_eq(i, 0); + test_eq(0L, tor_parse_long("-"TOOBIG, 10, LONG_MIN, LONG_MAX, &i, NULL)); + test_eq(i, 0); + test_eq(0UL, tor_parse_ulong(TOOBIG, 10, 0, ULONG_MAX, &i, NULL)); + test_eq(i, 0); + test_eq(U64_LITERAL(0), tor_parse_uint64(TOOBIG, 10, + 0, UINT64_MAX, &i, NULL)); + test_eq(i, 0); + } + /* Test failing snprintf cases */ test_eq(-1, tor_snprintf(buf, 0, "Foo")); test_eq(-1, tor_snprintf(buf, 2, "Foo"));

1 0

[tor/master] Merge branch 'bug5786_range_022' into maint-0.2.2
by nickm＠torproject.org 10 May '12

10 May '12

commit 79c4c8195a9da15dc13866dd4a706807c79dba46 Merge: 5bbf04d 9b34462 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu May 10 15:38:57 2012 -0400 Merge branch 'bug5786_range_022' into maint-0.2.2 changes/bug5786_range | 8 ++++++++ src/common/util.c | 7 +++++++ src/test/test_util.c | 15 +++++++++++++++ 3 files changed, 30 insertions(+), 0 deletions(-)

1 0