September 2014 - tor-commits - lists.torproject.org

[tor/master] Extra tests for tor_memeq and memcmp
by nickm＠torproject.org 11 Sep '14

11 Sep '14

commit 5126bc2ebd9ae67449e3c4f18828acb895d86308 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Sep 10 23:58:02 2014 -0400 Extra tests for tor_memeq and memcmp (Patch from teor; part of 13104) --- src/test/test_util.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/test/test_util.c b/src/test/test_util.c index 16a2927..f091fc9 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -3251,6 +3251,25 @@ test_util_di_ops(void) test_eq(neq1, !eq1); } + /* exhaustively white-box test tor_memeq + * against each possible (single-byte) bit difference + * some arithmetic bugs only appear with certain bit patterns */ + const uint8_t z = 0; + uint8_t ii = 0; + for (i = 0; i < 256; i++) { + ii = (uint8_t)i; + test_eq(tor_memeq(&z, &ii, 1), z == ii); + } + + /* exhaustively white-box test tor_memcmp + * against each possible single-byte numeric difference + * some arithmetic bugs only appear with certain bit patterns */ + for (i = 0; i < 256; i++) { + ii = (uint8_t)i; + test_eq(tor_memcmp(&z, &ii, 1) > 0 ? GT : EQ, z > ii ? GT : EQ); + test_eq(tor_memcmp(&ii, &z, 1) < 0 ? LT : EQ, ii < z ? LT : EQ); + } + tt_int_op(1, ==, safe_mem_is_zero("", 0)); tt_int_op(1, ==, safe_mem_is_zero("", 1)); tt_int_op(0, ==, safe_mem_is_zero("a", 1));

1 0

[tor/master] Add a changes file for bug 13104
by nickm＠torproject.org 11 Sep '14

11 Sep '14

commit 9e595a95a8aae83381246f012157107c9f6dde5e Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Sep 11 00:05:56 2014 -0400 Add a changes file for bug 13104 --- changes/bug13104 | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/changes/bug13104 b/changes/bug13104 new file mode 100644 index 0000000..331db64 --- /dev/null +++ b/changes/bug13104 @@ -0,0 +1,4 @@ + o Minor bugfixes: + - Fix several instances of possible integer overflow/underflow/NaN. + Fixes bug 13104; bugfix on 0.2.3.1-alpha and later. Patches from + "teor".

1 0

[tor/master] C90 compliance for #13104 fixes
by nickm＠torproject.org 11 Sep '14

11 Sep '14

commit 2491eadf002e993bee11aa76597ec7f361d8f6e5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Sep 11 00:10:53 2014 -0400 C90 compliance for #13104 fixes --- src/test/test_util.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/test_util.c b/src/test/test_util.c index eed37ca..e105435 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -3277,6 +3277,7 @@ test_util_di_ops(void) /* exhaustively white-box test tor_memeq * against each possible (single-byte) bit difference * some arithmetic bugs only appear with certain bit patterns */ + { const uint8_t z = 0; uint8_t ii = 0; for (i = 0; i < 256; i++) { @@ -3292,6 +3293,7 @@ test_util_di_ops(void) test_eq(tor_memcmp(&z, &ii, 1) > 0 ? GT : EQ, z > ii ? GT : EQ); test_eq(tor_memcmp(&ii, &z, 1) < 0 ? LT : EQ, ii < z ? LT : EQ); } + } tt_int_op(1, ==, safe_mem_is_zero("", 0)); tt_int_op(1, ==, safe_mem_is_zero("", 1));

1 0

[tor/master] Merge remote-tracking branch 'public/bug13104_025'
by nickm＠torproject.org 11 Sep '14

11 Sep '14

commit 48558ed1aa070ef121b339eef88e7822f8e45978 Merge: 73ee161 2491ead Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Sep 11 00:11:26 2014 -0400 Merge remote-tracking branch 'public/bug13104_025' changes/bug13104 | 4 + src/common/util.c | 31 ++++++-- src/or/routerlist.c | 5 +- src/test/test_dir.c | 27 +++++++ src/test/test_util.c | 211 ++++++++++++++++++++++++++++++++++++++++++++++---- 5 files changed, 256 insertions(+), 22 deletions(-) diff --cc src/or/routerlist.c index a018181,0df411b..1faa05f --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@@ -1804,10 -1802,10 +1804,10 @@@ scale_array_elements_to_u64(u64_dbl_t * uint64_t *total_out) { double total = 0.0; - double scale_factor; + double scale_factor = 0.0; int i; /* big, but far away from overflowing an int64_t */ -#define SCALE_TO_U64_MAX (INT64_MAX / 4) +#define SCALE_TO_U64_MAX ((int64_t) (INT64_MAX / 4)) for (i = 0; i < n_entries; ++i) total += entries[i].dbl;

1 0

[tor/master] Avoid overflows and underflows in sscanf and friends
by nickm＠torproject.org 11 Sep '14

11 Sep '14

commit d2463c0cfee066111c3a72d188cd897957aa2988 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Sep 10 23:57:31 2014 -0400 Avoid overflows and underflows in sscanf and friends (Patch from teor on 13104) --- src/common/util.c | 23 +++++-- src/test/test_util.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 171 insertions(+), 19 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 2d7893b..297ae78 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -2804,10 +2804,14 @@ scan_unsigned(const char **bufp, unsigned long *out, int width, int base) while (**bufp && (hex?TOR_ISXDIGIT(**bufp):TOR_ISDIGIT(**bufp)) && scanned_so_far < width) { int digit = hex?hex_decode_digit(*(*bufp)++):digit_to_num(*(*bufp)++); - unsigned long new_result = result * base + digit; - if (new_result < result) - return -1; /* over/underflow. */ - result = new_result; + // Check for overflow beforehand, without actually causing any overflow + // This preserves functionality on compilers that don't wrap overflow + // (i.e. that trap or optimise away overflow) + // result * base + digit > ULONG_MAX + // result * base > ULONG_MAX - digit + if (result > (ULONG_MAX - digit)/base) + return -1; /* Processing this digit would overflow */ + result = result * base + digit; ++scanned_so_far; } @@ -2842,10 +2846,17 @@ scan_signed(const char **bufp, long *out, int width) if (scan_unsigned(bufp, &result, width, 10) < 0) return -1; - if (neg) { + if (neg && result > 0) { if (result > ((unsigned long)LONG_MAX) + 1) return -1; /* Underflow */ - *out = -(long)result; + // Avoid overflow on the cast to signed long when result is LONG_MIN + // by subtracting 1 from the unsigned long positive value, + // then, after it has been cast to signed and negated, + // subtracting the original 1 (the double-subtraction is intentional). + // Otherwise, the cast to signed could cause a temporary long + // to equal LONG_MAX + 1, which is undefined. + // We avoid underflow on the subtraction by treating -0 as positive. + *out = (-(long)(result - 1)) - 1; } else { if (result > LONG_MAX) return -1; /* Overflow */ diff --git a/src/test/test_util.c b/src/test/test_util.c index 151ec69..16a2927 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -1672,6 +1672,7 @@ static void test_util_sscanf(void) { unsigned u1, u2, u3; + unsigned long ulng; char s1[20], s2[10], s3[10], ch; int r; long lng1,lng2; @@ -1713,11 +1714,6 @@ test_util_sscanf(void) test_eq(0, tor_sscanf("", "%u", &u1)); /* absent number */ test_eq(0, tor_sscanf("A", "%u", &u1)); /* bogus number */ test_eq(0, tor_sscanf("-1", "%u", &u1)); /* negative number */ - test_eq(1, tor_sscanf("4294967295", "%u", &u1)); /* UINT32_MAX should work */ - test_eq(4294967295u, u1); - test_eq(0, tor_sscanf("4294967296", "%u", &u1)); /* But not at 32 bits */ - test_eq(1, tor_sscanf("4294967296", "%9u", &u1)); /* but parsing only 9... */ - test_eq(429496729u, u1); /* Numbers with size (eg. %2u) */ test_eq(0, tor_sscanf("-1", "%2u", &u1)); @@ -1812,46 +1808,191 @@ test_util_sscanf(void) test_eq(int2, -1); #if SIZEOF_INT == 4 + /* %u */ + /* UINT32_MAX should work */ + test_eq(1, tor_sscanf("4294967295", "%u", &u1)); + test_eq(4294967295U, u1); + + /* But UINT32_MAX + 1 shouldn't work */ + test_eq(0, tor_sscanf("4294967296", "%u", &u1)); + /* but parsing only 9... */ + test_eq(1, tor_sscanf("4294967296", "%9u", &u1)); + test_eq(429496729U, u1); + + /* %x */ + /* UINT32_MAX should work */ + test_eq(1, tor_sscanf("FFFFFFFF", "%x", &u1)); + test_eq(0xFFFFFFFF, u1); + + /* But UINT32_MAX + 1 shouldn't work */ + test_eq(0, tor_sscanf("100000000", "%x", &u1)); + + /* %d */ + /* INT32_MIN and INT32_MAX should work */ r = tor_sscanf("-2147483648. 2147483647.", "%d. %d.", &int1, &int2); test_eq(r,2); - test_eq(int1, -2147483647-1); + test_eq(int1, -2147483647 - 1); test_eq(int2, 2147483647); - r = tor_sscanf("-2147483679.", "%d.", &int1); + /* But INT32_MIN - 1 and INT32_MAX + 1 shouldn't work */ + r = tor_sscanf("-2147483649.", "%d.", &int1); test_eq(r,0); - r = tor_sscanf("2147483678.", "%d.", &int1); + r = tor_sscanf("2147483648.", "%d.", &int1); + test_eq(r,0); + + /* and the first failure stops further processing */ + r = tor_sscanf("-2147483648. 2147483648.", + "%d. %d.", &int1, &int2); + test_eq(r,1); + + r = tor_sscanf("-2147483649. 2147483647.", + "%d. %d.", &int1, &int2); + test_eq(r,0); + + r = tor_sscanf("2147483648. -2147483649.", + "%d. %d.", &int1, &int2); test_eq(r,0); #elif SIZEOF_INT == 8 + /* %u */ + /* UINT64_MAX should work */ + test_eq(1, tor_sscanf("18446744073709551615", "%u", &u1)); + test_eq(18446744073709551615U, u1); + + /* But UINT64_MAX + 1 shouldn't work */ + test_eq(0, tor_sscanf("18446744073709551616", "%u", &u1)); + /* but parsing only 19... */ + test_eq(1, tor_sscanf("18446744073709551616", "%19u", &u1)); + test_eq(1844674407370955161U, u1); + + /* %x */ + /* UINT64_MAX should work */ + test_eq(1, tor_sscanf("FFFFFFFFFFFFFFFF", "%x", &u1)); + test_eq(0xFFFFFFFFFFFFFFFF, u1); + + /* But UINT64_MAX + 1 shouldn't work */ + test_eq(0, tor_sscanf("10000000000000000", "%x", &u1)); + + /* %d */ + /* INT64_MIN and INT64_MAX should work */ r = tor_sscanf("-9223372036854775808. 9223372036854775807.", "%d. %d.", &int1, &int2); test_eq(r,2); - test_eq(int1, -9223372036854775807-1); + test_eq(int1, -9223372036854775807 - 1); test_eq(int2, 9223372036854775807); + /* But INT64_MIN - 1 and INT64_MAX + 1 shouldn't work */ r = tor_sscanf("-9223372036854775809.", "%d.", &int1); test_eq(r,0); r = tor_sscanf("9223372036854775808.", "%d.", &int1); test_eq(r,0); + + /* and the first failure stops further processing */ + r = tor_sscanf("-9223372036854775808. 9223372036854775808.", + "%d. %d.", &int1, &int2); + test_eq(r,1); + + r = tor_sscanf("-9223372036854775809. 9223372036854775807.", + "%d. %d.", &int1, &int2); + test_eq(r,0); + + r = tor_sscanf("9223372036854775808. -9223372036854775809.", + "%d. %d.", &int1, &int2); + test_eq(r,0); #endif #if SIZEOF_LONG == 4 + /* %lu */ + /* UINT32_MAX should work */ + test_eq(1, tor_sscanf("4294967295", "%lu", &ulng)); + test_eq(4294967295UL, ulng); + + /* But UINT32_MAX + 1 shouldn't work */ + test_eq(0, tor_sscanf("4294967296", "%lu", &ulng)); + /* but parsing only 9... */ + test_eq(1, tor_sscanf("4294967296", "%9lu", &ulng)); + test_eq(429496729UL, ulng); + + /* %lx */ + /* UINT32_MAX should work */ + test_eq(1, tor_sscanf("FFFFFFFF", "%lx", &ulng)); + test_eq(0xFFFFFFFFUL, ulng); + + /* But UINT32_MAX + 1 shouldn't work */ + test_eq(0, tor_sscanf("100000000", "%lx", &ulng)); + + /* %ld */ + /* INT32_MIN and INT32_MAX should work */ r = tor_sscanf("-2147483648. 2147483647.", "%ld. %ld.", &lng1, &lng2); test_eq(r,2); - test_eq(lng1, -2147483647 - 1); - test_eq(lng2, 2147483647); + test_eq(lng1, -2147483647L - 1L); + test_eq(lng2, 2147483647L); + + /* But INT32_MIN - 1 and INT32_MAX + 1 shouldn't work */ + r = tor_sscanf("-2147483649.", "%ld.", &lng1); + test_eq(r,0); + + r = tor_sscanf("2147483648.", "%ld.", &lng1); + test_eq(r,0); + + /* and the first failure stops further processing */ + r = tor_sscanf("-2147483648. 2147483648.", + "%ld. %ld.", &lng1, &lng2); + test_eq(r,1); + + r = tor_sscanf("-2147483649. 2147483647.", + "%ld. %ld.", &lng1, &lng2); + test_eq(r,0); + + r = tor_sscanf("2147483648. -2147483649.", + "%ld. %ld.", &lng1, &lng2); + test_eq(r,0); #elif SIZEOF_LONG == 8 + /* %lu */ + /* UINT64_MAX should work */ + test_eq(1, tor_sscanf("18446744073709551615", "%lu", &ulng)); + test_eq(18446744073709551615UL, ulng); + + /* But UINT64_MAX + 1 shouldn't work */ + test_eq(0, tor_sscanf("18446744073709551616", "%lu", &ulng)); + /* but parsing only 19... */ + test_eq(1, tor_sscanf("18446744073709551616", "%19lu", &ulng)); + test_eq(1844674407370955161UL, ulng); + + /* %lx */ + /* UINT64_MAX should work */ + test_eq(1, tor_sscanf("FFFFFFFFFFFFFFFF", "%lx", &ulng)); + test_eq(0xFFFFFFFFFFFFFFFFUL, ulng); + + /* But UINT64_MAX + 1 shouldn't work */ + test_eq(0, tor_sscanf("10000000000000000", "%lx", &ulng)); + + /* %ld */ + /* INT64_MIN and INT64_MAX should work */ r = tor_sscanf("-9223372036854775808. 9223372036854775807.", "%ld. %ld.", &lng1, &lng2); test_eq(r,2); - test_eq(lng1, -9223372036854775807L - 1); + test_eq(lng1, -9223372036854775807L - 1L); test_eq(lng2, 9223372036854775807L); + /* But INT64_MIN - 1 and INT64_MAX + 1 shouldn't work */ + r = tor_sscanf("-9223372036854775809.", "%ld.", &lng1); + test_eq(r,0); + + r = tor_sscanf("9223372036854775808.", "%ld.", &lng1); + test_eq(r,0); + + /* and the first failure stops further processing */ r = tor_sscanf("-9223372036854775808. 9223372036854775808.", "%ld. %ld.", &lng1, &lng2); test_eq(r,1); - r = tor_sscanf("-9223372036854775809. 9223372036854775808.", + + r = tor_sscanf("-9223372036854775809. 9223372036854775807.", + "%ld. %ld.", &lng1, &lng2); + test_eq(r,0); + + r = tor_sscanf("9223372036854775808. -9223372036854775809.", "%ld. %ld.", &lng1, &lng2); test_eq(r,0); #endif

1 0

[tor/master] Merge remote-tracking branch 'origin/maint-0.2.5'
by nickm＠torproject.org 11 Sep '14

11 Sep '14

commit 73ee161d8a7bc06eefa58654cbfe421449921eec Merge: e07206a 3c2c6a6 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Sep 10 23:48:59 2014 -0400 Merge remote-tracking branch 'origin/maint-0.2.5' changes/bug13096 | 4 ++++ src/or/routerlist.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-)

1 0

[tor/master] Merge remote-tracking branch 'public/bug12908_025' into maint-0.2.5
by nickm＠torproject.org 11 Sep '14

11 Sep '14

commit a9b2e5eac6168a1eb09da8d6d3ff43b08cc51411 Merge: 8eed82b a3fe8b1 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Sep 10 22:12:47 2014 -0400 Merge remote-tracking branch 'public/bug12908_025' into maint-0.2.5 changes/bug12908 | 4 ++++ src/or/config.c | 7 +++++++ 2 files changed, 11 insertions(+)

1 0

[tor/master] In routerlist_assert_ok(), check r2 before taking &(r2->cache_info)
by nickm＠torproject.org 11 Sep '14

11 Sep '14

commit 3c2c6a61163cd6a42cc0eeee9fc43200b9f08503 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Sep 10 23:46:20 2014 -0400 In routerlist_assert_ok(), check r2 before taking &(r2->cache_info) Technically, we're not allowed to take the address of a member can't exist relative to the null pointer. That makes me wonder how any sane compliant system implements the offsetof macro, but let's let sleeping balrogs lie. Fixes 13096; patch on 0.1.1.9-alpha; patch from "teor", who was using clang -fsanitize=undefined-trap -fsanitize-undefined-trap-on-error -ftrapv --- changes/bug13096 | 4 ++++ src/or/routerlist.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/changes/bug13096 b/changes/bug13096 new file mode 100644 index 0000000..521faaf --- /dev/null +++ b/changes/bug13096 @@ -0,0 +1,4 @@ + o Minor bugfixes (conformance): + - In routerlist_assert_ok(), don't take the address of a routerinfo's + cache_info member unless that routerinfo is non-NULL. Fixes bug + 13096; bugfix on 0.1.1.9-alpha. Patch by "teor". diff --git a/src/or/routerlist.c b/src/or/routerlist.c index b5e9245..32cbe19 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -4938,7 +4938,7 @@ routerlist_assert_ok(const routerlist_t *rl) } SMARTLIST_FOREACH_END(r); SMARTLIST_FOREACH_BEGIN(rl->old_routers, signed_descriptor_t *, sd) { r2 = rimap_get(rl->identity_map, sd->identity_digest); - tor_assert(sd != &(r2->cache_info)); + tor_assert(!r2 || sd != &(r2->cache_info)); sd2 = sdmap_get(rl->desc_digest_map, sd->signed_descriptor_digest); tor_assert(sd == sd2); tor_assert(sd->routerlist_index == sd_sl_idx);

1 0

[tor/maint-0.2.5] In routerlist_assert_ok(), check r2 before taking &(r2->cache_info)
by nickm＠torproject.org 11 Sep '14

11 Sep '14

commit 3c2c6a61163cd6a42cc0eeee9fc43200b9f08503 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Sep 10 23:46:20 2014 -0400 In routerlist_assert_ok(), check r2 before taking &(r2->cache_info) Technically, we're not allowed to take the address of a member can't exist relative to the null pointer. That makes me wonder how any sane compliant system implements the offsetof macro, but let's let sleeping balrogs lie. Fixes 13096; patch on 0.1.1.9-alpha; patch from "teor", who was using clang -fsanitize=undefined-trap -fsanitize-undefined-trap-on-error -ftrapv --- changes/bug13096 | 4 ++++ src/or/routerlist.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/changes/bug13096 b/changes/bug13096 new file mode 100644 index 0000000..521faaf --- /dev/null +++ b/changes/bug13096 @@ -0,0 +1,4 @@ + o Minor bugfixes (conformance): + - In routerlist_assert_ok(), don't take the address of a routerinfo's + cache_info member unless that routerinfo is non-NULL. Fixes bug + 13096; bugfix on 0.1.1.9-alpha. Patch by "teor". diff --git a/src/or/routerlist.c b/src/or/routerlist.c index b5e9245..32cbe19 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -4938,7 +4938,7 @@ routerlist_assert_ok(const routerlist_t *rl) } SMARTLIST_FOREACH_END(r); SMARTLIST_FOREACH_BEGIN(rl->old_routers, signed_descriptor_t *, sd) { r2 = rimap_get(rl->identity_map, sd->identity_digest); - tor_assert(sd != &(r2->cache_info)); + tor_assert(!r2 || sd != &(r2->cache_info)); sd2 = sdmap_get(rl->desc_digest_map, sd->signed_descriptor_digest); tor_assert(sd == sd2); tor_assert(sd->routerlist_index == sd_sl_idx);

1 0

[tor/master] Merge remote-tracking branch 'yawning/bug_8402'
by nickm＠torproject.org 11 Sep '14

11 Sep '14

commit e07206afea28d110a0a7b74d4e230db3b0fe3120 Merge: 5474d8a cae4483 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Sep 10 23:41:55 2014 -0400 Merge remote-tracking branch 'yawning/bug_8402' changes/bug8402 | 5 +++ src/or/config.c | 13 ++++-- src/or/connection.c | 42 +++++++++++-------- src/or/transports.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++++-- src/or/transports.h | 6 +++ src/test/test_pt.c | 81 ++++++++++++++++++++++++++++++++++++ 6 files changed, 238 insertions(+), 24 deletions(-)

1 0