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

[tor/master] Make the two branches of tor_tls_used_v1_handshake into one.
by nickm＠torproject.org 21 Aug '14

21 Aug '14

commit 2cf229ab60b43c79289292f918f671ae6fec5da4 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Aug 21 10:12:54 2014 -0400 Make the two branches of tor_tls_used_v1_handshake into one. (Coverity thinks that "if (a) X; else X;" is probably a bug.) [Coverity CID 1232086] --- src/common/tortls.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/common/tortls.c b/src/common/tortls.c index 0f98968..5fe8d81 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -2611,16 +2611,20 @@ check_no_tls_errors_(const char *fname, int line) int tor_tls_used_v1_handshake(tor_tls_t *tls) { +#if defined(V2_HANDSHAKE_SERVER) && defined(V2_HANDSHAKE_CLIENT) + return ! tls->wasV2Handshake; +#else if (tls->isServer) { -#ifdef V2_HANDSHAKE_SERVER +# ifdef V2_HANDSHAKE_SERVER return ! tls->wasV2Handshake; -#endif +# endif } else { -#ifdef V2_HANDSHAKE_CLIENT +# ifdef V2_HANDSHAKE_CLIENT return ! tls->wasV2Handshake; -#endif +# endif } return 1; +#endif } /** Return true iff <b>name</b> is a DN of a kind that could only

1 0

[tor/master] Fix memory leaks in test_entrynodes.c
by nickm＠torproject.org 21 Aug '14

21 Aug '14

commit b6a725c67e46d274a518b6c5243fffdf1730a23f Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Aug 21 10:18:17 2014 -0400 Fix memory leaks in test_entrynodes.c [Coverity CID 1232087 and 1232088] --- src/test/test_entrynodes.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/test_entrynodes.c b/src/test/test_entrynodes.c index fbb7625..fab82c3 100644 --- a/src/test/test_entrynodes.c +++ b/src/test/test_entrynodes.c @@ -431,6 +431,7 @@ test_entry_guards_parse_state_simple(void *arg) done: state_lines_free(entry_state_lines); or_state_free(state); + tor_free(msg); } /** Similar to test_entry_guards_parse_state_simple() but aims to test @@ -515,6 +516,7 @@ test_entry_guards_parse_state_pathbias(void *arg) done: or_state_free(state); state_lines_free(entry_state_lines); + tor_free(msg); } /* Simple test of entry_guards_set_from_config() by specifying a

1 0

[tor/master] Explicitly cast when dividing ints then implicitly casting to double.
by nickm＠torproject.org 21 Aug '14

21 Aug '14

commit 2a0a5fe6123bd87f996814991641cc404601ea55 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Aug 21 10:19:26 2014 -0400 Explicitly cast when dividing ints then implicitly casting to double. Coverity thinks that when we do "double x = int1/int2;", we probably meant "double x = ((double)int1) / int2;". In these cases, we didn't. [Coverity CID 1232089 and 1232090] --- src/or/circuitstats.c | 4 +++- src/or/routerlist.c | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/or/circuitstats.c b/src/or/circuitstats.c index 5cdd534..88a1f9b 100644 --- a/src/or/circuitstats.c +++ b/src/or/circuitstats.c @@ -1371,10 +1371,11 @@ circuit_build_times_network_check_changed(circuit_build_times_t *cbt) } cbt->liveness.after_firsthop_idx = 0; +#define MAX_TIMEOUT ((int32_t) (INT32_MAX/2)) /* Check to see if this has happened before. If so, double the timeout * to give people on abysmally bad network connections a shot at access */ if (cbt->timeout_ms >= circuit_build_times_get_initial_timeout()) { - if (cbt->timeout_ms > INT32_MAX/2 || cbt->close_ms > INT32_MAX/2) { + if (cbt->timeout_ms > MAX_TIMEOUT || cbt->close_ms > MAX_TIMEOUT) { log_warn(LD_CIRC, "Insanely large circuit build timeout value. " "(timeout = %fmsec, close = %fmsec)", cbt->timeout_ms, cbt->close_ms); @@ -1386,6 +1387,7 @@ circuit_build_times_network_check_changed(circuit_build_times_t *cbt) cbt->close_ms = cbt->timeout_ms = circuit_build_times_get_initial_timeout(); } +#undef MAX_TIMEOUT cbt_control_event_buildtimeout_set(cbt, BUILDTIMEOUT_SET_EVENT_RESET); diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 12ed71d..14451c0 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -1809,7 +1809,7 @@ scale_array_elements_to_u64(u64_dbl_t *entries, int n_entries, double scale_factor; 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] Add a missing goto to an unusable branch and make the branch LD_BUG.
by nickm＠torproject.org 21 Aug '14

21 Aug '14

commit e6a05c1c549dca6e76b900feaee97941c5fb723c Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Aug 21 10:21:17 2014 -0400 Add a missing goto to an unusable branch and make the branch LD_BUG. (It's LD_BUG to reach this point because the hashed password values were tested earlier from options_validate) [Coverity CID 1232091] --- src/or/control.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/or/control.c b/src/or/control.c index a920ac8..11a8530 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -1160,9 +1160,10 @@ handle_control_authenticate(control_connection_t *conn, uint32_t len, } if (bad) { if (!also_cookie) { - log_warn(LD_CONTROL, + log_warn(LD_BUG, "Couldn't decode HashedControlPassword: invalid base16"); errstr="Couldn't decode HashedControlPassword value in configuration."; + goto err; } bad_password = 1; SMARTLIST_FOREACH(sl, char *, cp, tor_free(cp));

1 0

[tor/master] Remove a dead check for errmsg in handle_control_authenticate
by nickm＠torproject.org 21 Aug '14

21 Aug '14

commit c9cac69ac68d8cdc66d3592f0812988336a1cf8d Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Aug 21 10:27:43 2014 -0400 Remove a dead check for errmsg in handle_control_authenticate Coverity doesn't like doing NULL checks on things that can't be NULL; I like checking things where the logic for their not being NULL is nontrivial. Let's compromise, and make it obvious that this field can't be NULL. [Coverity CID 202004] --- src/or/control.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/or/control.c b/src/or/control.c index 11a8530..b3a9dd6 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -1039,7 +1039,7 @@ handle_control_authenticate(control_connection_t *conn, uint32_t len, { int used_quoted_string = 0; const or_options_t *options = get_options(); - const char *errstr = NULL; + const char *errstr = "Unknown error"; char *password; size_t password_len; const char *cp; @@ -1199,8 +1199,7 @@ handle_control_authenticate(control_connection_t *conn, uint32_t len, err: tor_free(password); - connection_printf_to_buf(conn, "515 Authentication failed: %s\r\n", - errstr ? errstr : "Unknown reason."); + connection_printf_to_buf(conn, "515 Authentication failed: %s\r\n", errstr); connection_mark_for_close(TO_CONN(conn)); return 0; ok:

1 0

[tor/master] Allow rend_service_intro_free to get called with NULL
by nickm＠torproject.org 21 Aug '14

21 Aug '14

commit 377b5c0510478994fccc4c8931cbce136eb41a5b Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Aug 21 10:34:29 2014 -0400 Allow rend_service_intro_free to get called with NULL (We allowed it previously, but produced an LD_BUG message when it happened, which is not consistent Also, remove inconsistent NULL checks before calling rend_service_intro_free. (Removing the check is for CID 718613) --- src/or/rendservice.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 629e42c..0633c35 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -1446,10 +1446,7 @@ rend_service_introduce(origin_circuit_t *circuit, const uint8_t *request, memwipe(hexcookie, 0, sizeof(hexcookie)); /* Free the parsed cell */ - if (parsed_req) { - rend_service_free_intro(parsed_req); - parsed_req = NULL; - } + rend_service_free_intro(parsed_req); /* Free rp if we must */ if (need_rp_free) extend_info_free(rp); @@ -1539,7 +1536,6 @@ void rend_service_free_intro(rend_intro_cell_t *request) { if (!request) { - log_info(LD_BUG, "rend_service_free_intro() called with NULL request!"); return; } @@ -1648,7 +1644,7 @@ rend_service_begin_parse_intro(const uint8_t *request, goto done; err: - if (rv) rend_service_free_intro(rv); + rend_service_free_intro(rv); rv = NULL; if (err_msg_out && !err_msg) { tor_asprintf(&err_msg,

1 0

[tor/master] Suppress coverity warning about overflowing in tor_memeq.
by nickm＠torproject.org 21 Aug '14

21 Aug '14

commit c43e45d0eaac67caa5b6c1449fb1e4931a410933 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Aug 21 10:44:13 2014 -0400 Suppress coverity warning about overflowing in tor_memeq. The unsigned underflow here is defined and intentional. CID 202482 --- src/common/di_ops.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/common/di_ops.c b/src/common/di_ops.c index 14a1443..f03b2b7 100644 --- a/src/common/di_ops.c +++ b/src/common/di_ops.c @@ -130,6 +130,7 @@ tor_memeq(const void *a, const void *b, size_t sz) * 1 & ((any_difference - 1) >> 8) == 0 */ + /*coverity[overflow]*/ return 1 & ((any_difference - 1) >> 8); }

1 0

[tor/master] Avoid performing an assert on an always-true value
by nickm＠torproject.org 21 Aug '14

21 Aug '14

commit 7bc25b5a78bfaa0b04eb55470a2b36cf9a466d72 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Aug 21 10:56:36 2014 -0400 Avoid performing an assert on an always-true value This was freaking out coverity. [CID 743379] --- src/common/container.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/common/container.c b/src/common/container.c index 54f041b..7f02dec 100644 --- a/src/common/container.c +++ b/src/common/container.c @@ -66,11 +66,17 @@ smartlist_ensure_capacity(smartlist_t *sl, int size) #define MAX_CAPACITY (INT_MAX) #else #define MAX_CAPACITY (int)((SIZE_MAX / (sizeof(void*)))) +#define ASSERT_CAPACITY #endif if (size > sl->capacity) { int higher = sl->capacity; if (PREDICT_UNLIKELY(size > MAX_CAPACITY/2)) { +#ifdef ASSERT_CAPACITY + /* We don't include this assertion when MAX_CAPACITY == INT_MAX, + * since int size; (size <= INT_MAX) makes analysis tools think we're + * doing something stupid. */ tor_assert(size <= MAX_CAPACITY); +#endif higher = MAX_CAPACITY; } else { while (size > higher) @@ -80,6 +86,8 @@ smartlist_ensure_capacity(smartlist_t *sl, int size) sl->list = tor_reallocarray(sl->list, sizeof(void *), ((size_t)sl->capacity)); } +#undef ASSERT_CAPACITY +#undef MAX_CAPACITY } /** Append element to the end of the list. */

1 0

[tor/master] Check return values for fcntl in tor_spawn_background.
by nickm＠torproject.org 21 Aug '14

21 Aug '14

commit 0de7565dfd4498e9bbc2ab295e5b160bab6e1867 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Aug 21 10:38:19 2014 -0400 Check return values for fcntl in tor_spawn_background. [CID 718609] --- src/common/util.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 4029512..16ff8e3 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -4052,8 +4052,11 @@ tor_spawn_background(const char *const filename, const char **argv, status = process_handle->status = PROCESS_STATUS_RUNNING; /* Set stdout/stderr pipes to be non-blocking */ - fcntl(process_handle->stdout_pipe, F_SETFL, O_NONBLOCK); - fcntl(process_handle->stderr_pipe, F_SETFL, O_NONBLOCK); + if (fcntl(process_handle->stdout_pipe, F_SETFL, O_NONBLOCK) < 0 || + fcntl(process_handle->stderr_pipe, F_SETFL, O_NONBLOCK) < 0) { + log_warn(LD_GENERAL, "Failed to set stderror/stdout pipes nonblocking " + "in parent process: %s", strerror(errno)); + } /* Open the buffered IO streams */ process_handle->stdout_handle = fdopen(process_handle->stdout_pipe, "r"); process_handle->stderr_handle = fdopen(process_handle->stderr_pipe, "r");

1 0

[tor/master] Mark one use of networkstatus_check_document_signature as (void)
by nickm＠torproject.org 21 Aug '14

21 Aug '14

commit a66fff6381abc5e3a70bd9cf3108560ed79f8d3e Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Aug 21 11:05:59 2014 -0400 Mark one use of networkstatus_check_document_signature as (void) Also explain why we aren't checking its return value. [CID 1198197] --- src/or/dirvote.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/or/dirvote.c b/src/or/dirvote.c index 6536b7f..cf6b88e 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -2273,8 +2273,11 @@ networkstatus_add_detached_signatures(networkstatus_t *target, if (!sig->good_signature && !sig->bad_signature) { cert = authority_cert_get_by_digests(sig->identity_digest, sig->signing_key_digest); - if (cert) - networkstatus_check_document_signature(target, sig, cert); + if (cert) { + /* Not checking the return value here, since we are going to look + * at the status of sig->good_signature in a moment. */ + (void) networkstatus_check_document_signature(target, sig, cert); + } } /* If this signature is good, or we don't have any signature yet,

1 0