August 2019 - tor-commits - lists.torproject.org

[tor/release-0.4.1] Modify "Protect buffers against INT_MAX datalen overflows." for 0.2.9
by teor＠torproject.org 09 Aug '19

09 Aug '19

commit 37bd7fa50d0901a87084b71299cc8c8786cd1cd8 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Apr 9 13:14:28 2019 -0400 Modify "Protect buffers against INT_MAX datalen overflows." for 0.2.9 --- src/or/buffers.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/or/buffers.c b/src/or/buffers.c index 394ba0ccb..b36e4ab50 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -394,10 +394,6 @@ buf_free(buf_t *buf) { if (!buf) return; - if (BUG(buf_out->datalen >= INT_MAX || buf_in->datalen >= INT_MAX)) - return; - if (BUG(buf_out->datalen >= INT_MAX - buf_in->datalen)) - return; buf_clear(buf); buf->magic = 0xdeadbeef; @@ -2067,4 +2063,3 @@ assert_buf_ok(buf_t *buf) tor_assert(buf->datalen == total); } } -

1 0

[tor/release-0.4.1] Merge remote-tracking branch 'tor-github/pr/1080' into maint-0.3.5
by teor＠torproject.org 09 Aug '19

09 Aug '19

commit 654769bcec3e98bea5e3ac48b2aabffcbe55375e Merge: 1ba2bd359 c8c2e2b8f Author: teor <teor(a)torproject.org> Date: Fri Aug 9 14:05:26 2019 +1000 Merge remote-tracking branch 'tor-github/pr/1080' into maint-0.3.5 changes/bug30781 | 4 ++++ src/feature/dirparse/routerparse.c | 3 +++ 2 files changed, 7 insertions(+)

1 0

[tor/release-0.2.9] dirparse: Stop crashing when parsing unknown descriptor purpose annotations
by teor＠torproject.org 09 Aug '19

09 Aug '19

commit ba83c1e5cf0e4ba0d63cb3728da059a9b241a161 Author: teor <teor(a)torproject.org> Date: Thu Jun 6 09:12:14 2019 +1000 dirparse: Stop crashing when parsing unknown descriptor purpose annotations We think this bug can only be triggered by modifying a local file. Fixes bug 30781; bugfix on 0.2.0.8-alpha. --- changes/bug30781 | 4 ++++ src/or/routerparse.c | 3 +++ 2 files changed, 7 insertions(+) diff --git a/changes/bug30781 b/changes/bug30781 new file mode 100644 index 000000000..7c7adf470 --- /dev/null +++ b/changes/bug30781 @@ -0,0 +1,4 @@ + o Minor bugfixes (directory authorities): + - Stop crashing after parsing an unknown descriptor purpose annotation. + We think this bug can only be triggered by modifying a local file. + Fixes bug 30781; bugfix on 0.2.0.8-alpha. diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 37d2d975f..f046cc39b 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -1921,6 +1921,9 @@ router_parse_entry_from_string(const char *s, const char *end, if ((tok = find_opt_by_keyword(tokens, A_PURPOSE))) { tor_assert(tok->n_args); router->purpose = router_purpose_from_string(tok->args[0]); + if (router->purpose == ROUTER_PURPOSE_UNKNOWN) { + goto err; + } } else { router->purpose = ROUTER_PURPOSE_GENERAL; }

1 0

[tor/release-0.2.9] Tweak comments in tor_vasprintf(), and add a changes file for 30651
by teor＠torproject.org 09 Aug '19

09 Aug '19

commit 0e0cf4abd80249faa23f2bbdb89e62ba96c898f0 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed May 29 09:38:32 2019 -0400 Tweak comments in tor_vasprintf(), and add a changes file for 30651 --- changes/bug30561 | 6 ++++++ src/common/compat.c | 10 ++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/changes/bug30561 b/changes/bug30561 new file mode 100644 index 000000000..afb3f02c6 --- /dev/null +++ b/changes/bug30561 @@ -0,0 +1,6 @@ + o Minor bugfixes (portability): + - Avoid crashing in our tor_vasprintf() implementation on systems that + define neither vasprintf() nor _vscprintf(). (This bug has been here + long enough that we question whether people are running Tor on such + systems, but we're applying the fix out of caution.) Fixes bug 30561; + bugfix on 0.2.8.2-alpha. Found and fixed by Tobias Stoeckmann. diff --git a/src/common/compat.c b/src/common/compat.c index d3bc2f5fe..ee3bf0fd5 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -554,13 +554,16 @@ tor_vasprintf(char **strp, const char *fmt, va_list args) * characters we need. We give it a try on a short buffer first, since * it might be nice to avoid the second vsnprintf call. */ + /* XXXX This code spent a number of years broken (see bug 30651). It is + * possible that no Tor users actually run on systems without vasprintf() or + * _vscprintf(). If so, we should consider removing this code. */ char buf[128]; int len, r; va_list tmp_args; va_copy(tmp_args, args); - /* Use vsnprintf to retrieve needed length. tor_vsnprintf() is not an option - * here because it will simply return -1 if buf is not large enough to hold the - * complete string. + /* Use vsnprintf to retrieve needed length. tor_vsnprintf() is not an + * option here because it will simply return -1 if buf is not large enough + * to hold the complete string. */ len = vsnprintf(buf, sizeof(buf), fmt, tmp_args); va_end(tmp_args); @@ -3550,4 +3553,3 @@ tor_get_avail_disk_space(const char *path) return -1; #endif } -

1 0

[tor/release-0.2.9] Merge remote-tracking branch 'tor-github/pr/1209' into combined31343_31374_029
by teor＠torproject.org 09 Aug '19

09 Aug '19

commit 7667c1cbaffeb4c1df9241d80d84d27f3f9bde18 Merge: 8d22c09ab 878f44090 Author: teor <teor(a)torproject.org> Date: Fri Aug 9 09:48:28 2019 +1000 Merge remote-tracking branch 'tor-github/pr/1209' into combined31343_31374_029 changes/bug31343 | 9 +++++++++ src/or/channeltls.c | 23 +++++++++++++++++++---- src/or/routerlist.c | 7 ++++--- 3 files changed, 32 insertions(+), 7 deletions(-)

1 0

[tor/release-0.2.9] Fixed tor_vasprintf on systems without vasprintf.
by teor＠torproject.org 09 Aug '19

09 Aug '19

commit 0d5a0b4f0ccc804913fbca20acf5fc62f52570b8 Author: Tobias Stoeckmann <tobias(a)stoeckmann.org> Date: Wed May 29 09:33:24 2019 -0400 Fixed tor_vasprintf on systems without vasprintf. If tor is compiled on a system with neither vasprintf nor _vscprintf, the fallback implementation exposes a logic flaw which prevents proper usage of strings longer than 127 characters: * tor_vsnprintf returns -1 if supplied buffer is not large enough, but tor_vasprintf uses this function to retrieve required length * the result of tor_vsnprintf is not properly checked for negative return values Both aspects together could in theory lead to exposure of uninitialized stack memory in the resulting string. This requires an invalid format string or data that exceeds integer limitations. Fortunately tor is not even able to run with this implementation because it runs into asserts early on during startup. Also the unit tests fail during a "make check" run. Signed-off-by: Tobias Stoeckmann <tobias(a)stoeckmann.org> [backported to 0.2.9 by nickm] --- src/common/compat.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/common/compat.c b/src/common/compat.c index 975875112..d3bc2f5fe 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -558,10 +558,17 @@ tor_vasprintf(char **strp, const char *fmt, va_list args) int len, r; va_list tmp_args; va_copy(tmp_args, args); - /* vsnprintf() was properly checked but tor_vsnprintf() available so - * why not use it? */ - len = tor_vsnprintf(buf, sizeof(buf), fmt, tmp_args); + /* Use vsnprintf to retrieve needed length. tor_vsnprintf() is not an option + * here because it will simply return -1 if buf is not large enough to hold the + * complete string. + */ + len = vsnprintf(buf, sizeof(buf), fmt, tmp_args); va_end(tmp_args); + buf[sizeof(buf) - 1] = '\0'; + if (len < 0) { + *strp = NULL; + return -1; + } if (len < (int)sizeof(buf)) { *strp = tor_strdup(buf); return len;

1 0

[tor/release-0.2.9] Avoid using labs() on time_t in channeltls.c
by teor＠torproject.org 09 Aug '19

09 Aug '19

commit 0849d2a2fdaeea2871f32bed35d410f19703aae1 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Aug 6 11:11:06 2019 -0400 Avoid using labs() on time_t in channeltls.c On some windows builds, time_t is 64 bits but long is not. This is causing appveyor builds to fail. Also, one of our uses of labs() on time_t was logically incorrect: it was telling us to accept NETINFO cells up to three minutes _before_ the message they were responding to, which doesn't make sense. This patch adds a time_abs() function that we should eventually move to intmath.h or something. For now, though, it will make merges easier to have it file-local in channeltls.c. Fixes bug 31343; bugfix on 0.2.4.4-alpha. --- changes/bug31343 | 9 +++++++++ src/or/channeltls.c | 23 +++++++++++++++++++---- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/changes/bug31343 b/changes/bug31343 new file mode 100644 index 000000000..17a8057ea --- /dev/null +++ b/changes/bug31343 @@ -0,0 +1,9 @@ + o Minor bugfixes (compilation): + - Avoid using labs() on time_t, which can cause compilation warnings + on 64-bit Windows builds. Fixes bug 31343; bugfix on 0.2.4.4-alpha. + + o Minor bugfixes (clock skew detection): + - Don't believe clock skew results from NETINFO cells that appear to + arrive before the VERSIONS cells they are responding to were sent. + Previously, we would accept them up to 3 minutes "in the past". + Fixes bug 31343; bugfix on 0.2.4.4-alpha. diff --git a/src/or/channeltls.c b/src/or/channeltls.c index 3a352d47f..ea69792f1 100644 --- a/src/or/channeltls.c +++ b/src/or/channeltls.c @@ -1584,6 +1584,18 @@ channel_tls_process_versions_cell(var_cell_t *cell, channel_tls_t *chan) } /** + * Helper: compute the absolute value of a time_t. + * + * (we need this because labs() doesn't always work for time_t, since + * long can be shorter than time_t.) + */ +static inline time_t +time_abs(time_t val) +{ + return (val < 0) ? -val : val; +} + +/** * Process a 'netinfo' cell * * This function is called to handle an incoming NETINFO cell; read and act @@ -1601,7 +1613,7 @@ channel_tls_process_netinfo_cell(cell_t *cell, channel_tls_t *chan) uint8_t n_other_addrs; time_t now = time(NULL); - long apparent_skew = 0; + time_t apparent_skew = 0; tor_addr_t my_apparent_addr = TOR_ADDR_NULL; tor_assert(cell); @@ -1659,7 +1671,11 @@ channel_tls_process_netinfo_cell(cell_t *cell, channel_tls_t *chan) /* Decode the cell. */ timestamp = ntohl(get_uint32(cell->payload)); - if (labs(now - chan->conn->handshake_state->sent_versions_at) < 180) { + const time_t sent_versions_at = + chan->conn->handshake_state->sent_versions_at; + if (now > sent_versions_at && (now - sent_versions_at) < 180) { + /* If we have gotten the NETINFO cell reasonably soon after having + * sent our VERSIONS cell, maybe we can learn skew information from it. */ apparent_skew = now - timestamp; } @@ -1705,7 +1721,7 @@ channel_tls_process_netinfo_cell(cell_t *cell, channel_tls_t *chan) /* Act on apparent skew. */ /** Warn when we get a netinfo skew with at least this value. */ #define NETINFO_NOTICE_SKEW 3600 - if (labs(apparent_skew) > NETINFO_NOTICE_SKEW && + if (time_abs(apparent_skew) && router_get_by_id_digest(chan->conn->identity_digest)) { int trusted = router_digest_is_trusted_dir(chan->conn->identity_digest); clock_skew_warning(TO_CONN(chan->conn), apparent_skew, trusted, LD_GENERAL, @@ -2182,4 +2198,3 @@ channel_tls_process_authenticate_cell(var_cell_t *cell, channel_tls_t *chan) #undef ERR } -

1 0

[tor/release-0.2.9] Restore proper behavior of netinfo skew check
by teor＠torproject.org 09 Aug '19

09 Aug '19

commit cd6cb453720a5300d00d7996c5b3a03f054cd293 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Aug 8 09:15:42 2019 -0400 Restore proper behavior of netinfo skew check My previous fix removed a comparison, which would have caused us to warn about every skew instead of skews of over an hour. --- src/or/channeltls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/or/channeltls.c b/src/or/channeltls.c index ea69792f1..d44f71913 100644 --- a/src/or/channeltls.c +++ b/src/or/channeltls.c @@ -1721,7 +1721,7 @@ channel_tls_process_netinfo_cell(cell_t *cell, channel_tls_t *chan) /* Act on apparent skew. */ /** Warn when we get a netinfo skew with at least this value. */ #define NETINFO_NOTICE_SKEW 3600 - if (time_abs(apparent_skew) && + if (time_abs(apparent_skew) > NETINFO_NOTICE_SKEW && router_get_by_id_digest(chan->conn->identity_digest)) { int trusted = router_digest_is_trusted_dir(chan->conn->identity_digest); clock_skew_warning(TO_CONN(chan->conn), apparent_skew, trusted, LD_GENERAL,

1 0

[tor/release-0.2.9] Fix another time_t/long warning for 31343.
by teor＠torproject.org 09 Aug '19

09 Aug '19

commit 878f4409015f741c7075d0ccf3da794a6f313302 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Aug 8 09:38:03 2019 -0400 Fix another time_t/long warning for 31343. --- src/or/routerlist.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/or/routerlist.c b/src/or/routerlist.c index f73ec9baa..f3b298006 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -5443,7 +5443,7 @@ int router_differences_are_cosmetic(const routerinfo_t *r1, const routerinfo_t *r2) { time_t r1pub, r2pub; - long time_difference; + time_t time_difference; tor_assert(r1 && r2); /* r1 should be the one that was published first. */ @@ -5506,7 +5506,9 @@ router_differences_are_cosmetic(const routerinfo_t *r1, const routerinfo_t *r2) * give or take some slop? */ r1pub = r1->cache_info.published_on; r2pub = r2->cache_info.published_on; - time_difference = labs(r2->uptime - (r1->uptime + (r2pub - r1pub))); + time_difference = r2->uptime - (r1->uptime + (r2pub - r1pub)); + if (time_difference < 0) + time_difference = - time_difference; if (time_difference > ROUTER_ALLOW_UPTIME_DRIFT && time_difference > r1->uptime * .05 && time_difference > r2->uptime * .05) @@ -5816,4 +5818,3 @@ refresh_all_country_info(void) nodelist_refresh_countries(); } -

1 0

[tor/release-0.2.9] Fix a warning about casting the results of GetProcAddress.
by teor＠torproject.org 09 Aug '19

09 Aug '19

commit 3a280b35ee45a1e4f4edaa3891a13d449d87fa8b Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Aug 8 11:43:42 2019 -0400 Fix a warning about casting the results of GetProcAddress. Fixes bug 31374; bugfix on 0.2.9.1-alpha. --- changes/ticket31374 | 4 ++++ src/common/compat_time.c | 3 +-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/changes/ticket31374 b/changes/ticket31374 new file mode 100644 index 000000000..e8eef9cd4 --- /dev/null +++ b/changes/ticket31374 @@ -0,0 +1,4 @@ + o Minor bugfixes (compilation warning): + - Fix a compilation warning on Windows about casting a function + pointer for GetTickCount64(). Fixes bug 31374; bugfix on + 0.2.9.1-alpha. diff --git a/src/common/compat_time.c b/src/common/compat_time.c index d044bbe1d..52da609db 100644 --- a/src/common/compat_time.c +++ b/src/common/compat_time.c @@ -443,7 +443,7 @@ monotime_init_internal(void) HANDLE h = load_windows_system_library(TEXT("kernel32.dll")); if (h) { - GetTickCount64_fn = (GetTickCount64_fn_t) + GetTickCount64_fn = (GetTickCount64_fn_t) (void(*)(void)) GetProcAddress(h, "GetTickCount64"); } // FreeLibrary(h) ? @@ -654,4 +654,3 @@ monotime_coarse_absolute_msec(void) return monotime_coarse_absolute_nsec() / ONE_MILLION; } #endif -

1 0