From b792afa91901abaa04417051d1369fa8e643d90d Mon Sep 17 00:00:00 2001 From: Sebastian Hahn Date: Sun, 13 Sep 2009 21:47:55 +0200 Subject: [PATCH 1/2] Fix a memory leak when parsing a ns Adding the same vote to a networkstatus consensus leads to a memory leak on the client side. Fix that by only using the first vote from any given voter, and ignoring the others. Problem found by Rotor, who also helped writing the patch. Thanks! --- ChangeLog | 3 +++ src/or/routerparse.c | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/ChangeLog b/ChangeLog index 1da6a97193..aabe9b4fbe 100644 --- a/ChangeLog +++ b/ChangeLog @@ -6,6 +6,9 @@ Changes in version 0.2.1.20 - 2009-??-?? patch. Bugfix on the 54th commit on Tor -- from July 2002, before the release of Tor 0.0.0. This is the new winner of the oldest-bug prize. + - Fix a remotely triggerable memory leak when a consensus document + contains more than one signature from the same voter. Bugfix on + 0.2.0.3-alpha. o New directory authorities: - Set up urras (run by Jacob Appelbaum) as the seventh v3 directory diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 97dd20f4e3..189458ee1e 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -2509,6 +2509,14 @@ networkstatus_parse_vote_from_string(const char *s, const char **eos_out, } else { if (tok->object_size >= INT_MAX) goto err; + /* We already parsed a vote from this voter. Use the first one. */ + if (v->signature) { + log_fn(LOG_PROTOCOL_WARN, LD_DIR, "We received a networkstatus " + "that contains two votes from the same voter. Ignoring " + "the second vote."); + continue; + } + v->signature = tor_memdup(tok->object_body, tok->object_size); v->signature_len = (int) tok->object_size; } From 113ba0e7270147b6eed10668970e1719139c4f27 Mon Sep 17 00:00:00 2001 From: Sebastian Hahn Date: Fri, 11 Sep 2009 15:40:09 +0200 Subject: [PATCH 2/2] make some bug 1090 warnings go away When we excluded some Exits, we were sometimes warning the user that we were going to use the node regardless. Many of those warnings were in fact bogus, because the relay in question was not used to connect to the outside world. Based on patch by Rotor, thanks! --- ChangeLog | 4 ++++ src/or/circuitbuild.c | 46 ++++++++++++++++++++----------------------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/ChangeLog b/ChangeLog index 1da6a97193..039cf5e162 100644 --- a/ChangeLog +++ b/ChangeLog @@ -33,6 +33,10 @@ Changes in version 0.2.1.20 - 2009-??-?? controller when we want to inform the user of overall success or overall failure. Bugfix on 0.1.2.6-alpha. Fixes bug 1075. Reported by SwissTorExit. + - Don't warn when we're using a circuit that ends with a node + excluded in ExcludeExitNodes, but the circuit is not used to access + the outside world. This should help fix bug 1090. Bugfix on + 0.2.1.6-alpha. o Minor features: - Add a "getinfo status/accepted-server-descriptor" controller diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 2d20efae3d..983eb6dac1 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1444,13 +1444,16 @@ choose_good_exit_server(uint8_t purpose, routerlist_t *dir, /** Log a warning if the user specified an exit for the circuit that * has been excluded from use by ExcludeNodes or ExcludeExitNodes. */ static void -warn_if_last_router_excluded(uint8_t purpose, const extend_info_t *exit) +warn_if_last_router_excluded(origin_circuit_t *circ, const extend_info_t *exit) { or_options_t *options = get_options(); routerset_t *rs = options->ExcludeNodes; const char *description; - int severity; int domain = LD_CIRC; + uint8_t purpose = circ->_base.purpose; + + if (circ->build_state->onehop_tunnel) + return; switch (purpose) { @@ -1463,48 +1466,40 @@ warn_if_last_router_excluded(uint8_t purpose, const extend_info_t *exit) (int)purpose); return; case CIRCUIT_PURPOSE_C_GENERAL: + if (circ->build_state->is_internal) + return; description = "Requested exit node"; rs = options->_ExcludeExitNodesUnion; - severity = LOG_WARN; break; case CIRCUIT_PURPOSE_C_INTRODUCING: case CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT: case CIRCUIT_PURPOSE_C_INTRODUCE_ACKED: - description = "Introduction point for hidden service"; - severity = LOG_INFO; - break; + case CIRCUIT_PURPOSE_S_ESTABLISH_INTRO: + case CIRCUIT_PURPOSE_S_CONNECT_REND: + case CIRCUIT_PURPOSE_S_REND_JOINED: + case CIRCUIT_PURPOSE_TESTING: + return; case CIRCUIT_PURPOSE_C_ESTABLISH_REND: case CIRCUIT_PURPOSE_C_REND_READY: case CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED: case CIRCUIT_PURPOSE_C_REND_JOINED: description = "Chosen rendezvous point"; - severity = LOG_WARN; domain = LD_BUG; break; - case CIRCUIT_PURPOSE_S_ESTABLISH_INTRO: - description = "Chosen introduction point"; - severity = LOG_INFO; - break; - case CIRCUIT_PURPOSE_S_CONNECT_REND: - case CIRCUIT_PURPOSE_S_REND_JOINED: - description = "Client-selected rendezvous point"; - severity = LOG_INFO; - break; - case CIRCUIT_PURPOSE_TESTING: - description = "Target for testing circuit"; - severity = LOG_INFO; - break; case CIRCUIT_PURPOSE_CONTROLLER: rs = options->_ExcludeExitNodesUnion; description = "Controller-selected circuit target"; - severity = LOG_WARN; break; } - if (routerset_contains_extendinfo(rs, exit)) - log_fn(severity, domain, "%s '%s' is in ExcludeNodes%s. Using anyway.", + if (routerset_contains_extendinfo(rs, exit)) { + log_fn(LOG_WARN, domain, "%s '%s' is in ExcludeNodes%s. Using anyway " + "(circuit purpose %d).", description,exit->nickname, - rs==options->ExcludeNodes?"":" or ExcludeExitNodes."); + rs==options->ExcludeNodes?"":" or ExcludeExitNodes", + (int)purpose); + circuit_log_path(LOG_WARN, domain, circ); + } return; } @@ -1529,7 +1524,7 @@ onion_pick_cpath_exit(origin_circuit_t *circ, extend_info_t *exit) } if (exit) { /* the circuit-builder pre-requested one */ - warn_if_last_router_excluded(circ->_base.purpose, exit); + warn_if_last_router_excluded(circ, exit); log_info(LD_CIRC,"Using requested exit node '%s'", exit->nickname); exit = extend_info_dup(exit); } else { /* we have to decide one */ @@ -1576,6 +1571,7 @@ int circuit_extend_to_new_exit(origin_circuit_t *circ, extend_info_t *exit) { int err_reason = 0; + warn_if_last_router_excluded(circ, exit); circuit_append_new_exit(circ, exit); circuit_set_state(TO_CIRCUIT(circ), CIRCUIT_STATE_BUILDING); if ((err_reason = circuit_send_next_onion_skin(circ))<0) {