March 2013 - tor-commits - lists.torproject.org

[tor/maint-0.2.4] Merge remote-tracking branch 'public/no_dup_guards' into maint-0.2.4
by nickm＠torproject.org 18 Mar '13

18 Mar '13

commit 26639b7798b5f5dae93e40bd7d050cb218a8aad2 Merge: 173efa1 4409531 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Mar 18 14:50:01 2013 -0400 Merge remote-tracking branch 'public/no_dup_guards' into maint-0.2.4 changes/bug8231 | 5 +++++ src/or/circuitbuild.c | 1 + src/or/entrynodes.c | 31 ++++++++++++++++++++++++++++++- src/or/or.h | 7 +++++++ src/or/routerlist.c | 3 +++ 5 files changed, 46 insertions(+), 1 deletions(-)

1 0

[tor/master] Fix a framing bug when reading versions from a versions cell.
by nickm＠torproject.org 18 Mar '13

18 Mar '13

commit b9037521c6ba333178c3f3197c39be360aba229c Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Mar 11 12:29:28 2013 -0400 Fix a framing bug when reading versions from a versions cell. Our ++ should have been += 2. This means that we'd accept version numbers even when they started at an odd position. This bug should be harmless in practice for so long as every version number we allow begins with a 0 byte, but if we ever have a version number starting with 1, 2, 3, or 4, there will be trouble here. Fix for bug 8059, reported pseudonymously. Bugfix on 0.2.0.10-alpha -- specifically, commit 6fcda529, where during development I increased the width of a version to 16 bits without changing the loop step. --- changes/bug8059 | 6 ++++++ src/or/channeltls.c | 2 +- 2 files changed, 7 insertions(+), 1 deletions(-) diff --git a/changes/bug8059 b/changes/bug8059 new file mode 100644 index 0000000..47273ed --- /dev/null +++ b/changes/bug8059 @@ -0,0 +1,6 @@ + o Minor bugfixes (protocol conformance): + - Fix a misframing issue when reading the version numbers in a + VERSIONS cell. Previously we would recognize [00 01 00 02] as + 'version 1, version 2, and version 0x100', when it should have + only included versions 1 and 2. Fixes bug 8059; bugfix on + 0.2.0.10-alpha. Reported pseudonymously. diff --git a/src/or/channeltls.c b/src/or/channeltls.c index 1035a14..60693da 100644 --- a/src/or/channeltls.c +++ b/src/or/channeltls.c @@ -1208,7 +1208,7 @@ channel_tls_process_versions_cell(var_cell_t *cell, channel_tls_t *chan) tor_assert(chan->conn->handshake_state); end = cell->payload + cell->payload_len; - for (cp = cell->payload; cp+1 < end; ++cp) { + for (cp = cell->payload; cp+1 < end; cp += 2) { uint16_t v = ntohs(get_uint16(cp)); if (is_or_protocol_version_known(v) && v > highest_supported_version) highest_supported_version = v;

1 0

[tor/master] Merge remote-tracking branch 'public/bug8059' into maint-0.2.4
by nickm＠torproject.org 18 Mar '13

18 Mar '13

commit 173efa10515e23b45e37f96a3755a2f16411a71a Merge: 57ffef3 b903752 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Mar 18 14:31:50 2013 -0400 Merge remote-tracking branch 'public/bug8059' into maint-0.2.4 changes/bug8059 | 6 ++++++ src/or/channeltls.c | 2 +- 2 files changed, 7 insertions(+), 1 deletions(-)

1 0

[tor/master] Merge remote-tracking branch 'origin/maint-0.2.4'
by nickm＠torproject.org 18 Mar '13

18 Mar '13

commit 15127476561a18a7074d8022f7b80ddfabbdc29b Merge: a770e1c 173efa1 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Mar 18 14:32:18 2013 -0400 Merge remote-tracking branch 'origin/maint-0.2.4' changes/bug8059 | 6 ++++++ src/or/channeltls.c | 2 +- 2 files changed, 7 insertions(+), 1 deletions(-)

1 0

[tor/maint-0.2.4] Merge remote-tracking branch 'public/bug8059' into maint-0.2.4
by nickm＠torproject.org 18 Mar '13

18 Mar '13

commit 173efa10515e23b45e37f96a3755a2f16411a71a Merge: 57ffef3 b903752 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Mar 18 14:31:50 2013 -0400 Merge remote-tracking branch 'public/bug8059' into maint-0.2.4 changes/bug8059 | 6 ++++++ src/or/channeltls.c | 2 +- 2 files changed, 7 insertions(+), 1 deletions(-)

1 0

[tor/maint-0.2.4] Fix a framing bug when reading versions from a versions cell.
by nickm＠torproject.org 18 Mar '13

18 Mar '13

commit b9037521c6ba333178c3f3197c39be360aba229c Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Mar 11 12:29:28 2013 -0400 Fix a framing bug when reading versions from a versions cell. Our ++ should have been += 2. This means that we'd accept version numbers even when they started at an odd position. This bug should be harmless in practice for so long as every version number we allow begins with a 0 byte, but if we ever have a version number starting with 1, 2, 3, or 4, there will be trouble here. Fix for bug 8059, reported pseudonymously. Bugfix on 0.2.0.10-alpha -- specifically, commit 6fcda529, where during development I increased the width of a version to 16 bits without changing the loop step. --- changes/bug8059 | 6 ++++++ src/or/channeltls.c | 2 +- 2 files changed, 7 insertions(+), 1 deletions(-) diff --git a/changes/bug8059 b/changes/bug8059 new file mode 100644 index 0000000..47273ed --- /dev/null +++ b/changes/bug8059 @@ -0,0 +1,6 @@ + o Minor bugfixes (protocol conformance): + - Fix a misframing issue when reading the version numbers in a + VERSIONS cell. Previously we would recognize [00 01 00 02] as + 'version 1, version 2, and version 0x100', when it should have + only included versions 1 and 2. Fixes bug 8059; bugfix on + 0.2.0.10-alpha. Reported pseudonymously. diff --git a/src/or/channeltls.c b/src/or/channeltls.c index 1035a14..60693da 100644 --- a/src/or/channeltls.c +++ b/src/or/channeltls.c @@ -1208,7 +1208,7 @@ channel_tls_process_versions_cell(var_cell_t *cell, channel_tls_t *chan) tor_assert(chan->conn->handshake_state); end = cell->payload + cell->payload_len; - for (cp = cell->payload; cp+1 < end; ++cp) { + for (cp = cell->payload; cp+1 < end; cp += 2) { uint16_t v = ntohs(get_uint16(cp)); if (is_or_protocol_version_known(v) && v > highest_supported_version) highest_supported_version = v;

1 0

[tor/master] Merge branch 'bug8206_squashed' into maint-0.2.4
by nickm＠torproject.org 18 Mar '13

18 Mar '13

commit 3894efc7601c17718a1ea2d228c0d841472104d8 Merge: eb94200 63b6757 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Mar 18 14:29:12 2013 -0400 Merge branch 'bug8206_squashed' into maint-0.2.4 changes/bug6206 | 6 +++++ src/common/compat.c | 54 ++++++++++++++++++++++++++++++++++++++++++-------- src/common/compat.h | 2 +- src/ext/eventdns.c | 8 ++++++- src/or/connection.c | 23 +++++++++++++++++---- src/or/cpuworker.c | 7 ++++- 6 files changed, 82 insertions(+), 18 deletions(-)

1 0

[tor/master] Check return values from fcntl and setsockopt
by nickm＠torproject.org 18 Mar '13

18 Mar '13

commit 63b67577d6df1080e0bca89d66a2e1550da6265d Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Mar 11 20:58:28 2013 -0400 Check return values from fcntl and setsockopt (Based on a patch from flupzor; bug #8206) --- changes/bug6206 | 6 +++++ src/common/compat.c | 54 ++++++++++++++++++++++++++++++++++++++++++-------- src/common/compat.h | 2 +- src/ext/eventdns.c | 8 ++++++- src/or/connection.c | 23 +++++++++++++++++---- src/or/cpuworker.c | 7 ++++- 6 files changed, 82 insertions(+), 18 deletions(-) diff --git a/changes/bug6206 b/changes/bug6206 new file mode 100644 index 0000000..61a16d2 --- /dev/null +++ b/changes/bug6206 @@ -0,0 +1,6 @@ + o Minor bugfixes: + - Always check the return values of functions fcntl() and + setsockopt(). We don't believe these are ever actually failing in + practice, but better safe than sorry. Also, checking these return + values should please some analysis tools (like Coverity). Patch + from 'flupzor'. Fix for bug 8206; bugfix on all versions of Tor. diff --git a/src/common/compat.c b/src/common/compat.c index d7ce894..25df9a9 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -137,8 +137,13 @@ tor_open_cloexec(const char *path, int flags, unsigned mode) fd = open(path, flags, mode); #ifdef FD_CLOEXEC - if (fd >= 0) - fcntl(fd, F_SETFD, FD_CLOEXEC); + if (fd >= 0) { + if (fcntl(fd, F_SETFD, FD_CLOEXEC) == -1) { + log_warn(LD_FS,"Couldn't set FD_CLOEXEC: %s", strerror(errno)); + close(fd); + return -1; + } + } #endif return fd; } @@ -150,8 +155,13 @@ tor_fopen_cloexec(const char *path, const char *mode) { FILE *result = fopen(path, mode); #ifdef FD_CLOEXEC - if (result != NULL) - fcntl(fileno(result), F_SETFD, FD_CLOEXEC); + if (result != NULL) { + if (fcntl(fileno(result), F_SETFD, FD_CLOEXEC) == -1) { + log_warn(LD_FS,"Couldn't set FD_CLOEXEC: %s", strerror(errno)); + fclose(result); + return NULL; + } + } #endif return result; } @@ -1024,7 +1034,15 @@ tor_open_socket(int domain, int type, int protocol) return s; #if defined(FD_CLOEXEC) - fcntl(s, F_SETFD, FD_CLOEXEC); + if (fcntl(s, F_SETFD, FD_CLOEXEC) == -1) { + log_warn(LD_FS,"Couldn't set FD_CLOEXEC: %s", strerror(errno)); +#if defined(_WIN32) + closesocket(s); +#else + close(s); +#endif + return -1; + } #endif goto socket_ok; /* So that socket_ok will not be unused. */ @@ -1059,7 +1077,11 @@ tor_accept_socket(tor_socket_t sockfd, struct sockaddr *addr, socklen_t *len) return s; #if defined(FD_CLOEXEC) - fcntl(s, F_SETFD, FD_CLOEXEC); + if (fcntl(s, F_SETFD, FD_CLOEXEC) == -1) { + log_warn(LD_NET, "Couldn't set FD_CLOEXEC: %s", strerror(errno)); + close(s); + return TOR_INVALID_SOCKET; + } #endif goto socket_ok; /* So that socket_ok will not be unused. */ @@ -1083,17 +1105,31 @@ get_n_open_sockets(void) return n; } -/** Turn <b>socket</b> into a nonblocking socket. +/** Turn <b>socket</b> into a nonblocking socket. Return 0 on success, -1 + * on failure. */ -void +int set_socket_nonblocking(tor_socket_t socket) { #if defined(_WIN32) unsigned long nonblocking = 1; ioctlsocket(socket, FIONBIO, (unsigned long*) &nonblocking); #else - fcntl(socket, F_SETFL, O_NONBLOCK); + int flags; + + flags = fcntl(socket, F_GETFL, 0); + if (flags == -1) { + log_warn(LD_NET, "Couldn't get file status flags: %s", strerror(errno)); + return -1; + } + flags |= O_NONBLOCK; + if (fcntl(socket, F_SETFL, flags) == -1) { + log_warn(LD_NET, "Couldn't set file status flags: %s", strerror(errno)); + return -1; + } #endif + + return 0; } /** diff --git a/src/common/compat.h b/src/common/compat.h index f9eb4ba..f0a34aa 100644 --- a/src/common/compat.h +++ b/src/common/compat.h @@ -518,7 +518,7 @@ int tor_inet_aton(const char *cp, struct in_addr *addr) ATTR_NONNULL((1,2)); const char *tor_inet_ntop(int af, const void *src, char *dst, size_t len); int tor_inet_pton(int af, const char *src, void *dst); int tor_lookup_hostname(const char *name, uint32_t *addr) ATTR_NONNULL((1,2)); -void set_socket_nonblocking(tor_socket_t socket); +int set_socket_nonblocking(tor_socket_t socket); int tor_socketpair(int family, int type, int protocol, tor_socket_t fd[2]); int network_init(void); diff --git a/src/ext/eventdns.c b/src/ext/eventdns.c index 3ee9f72..0dd7629 100644 --- a/src/ext/eventdns.c +++ b/src/ext/eventdns.c @@ -2275,6 +2275,7 @@ _evdns_nameserver_add_impl(const struct sockaddr *address, const struct nameserver *server = server_head, *const started_at = server_head; struct nameserver *ns; + int flags; int err = 0; if (server) { @@ -2306,7 +2307,12 @@ _evdns_nameserver_add_impl(const struct sockaddr *address, ioctlsocket(ns->socket, FIONBIO, &nonblocking); } #else - fcntl(ns->socket, F_SETFL, O_NONBLOCK); + if (fcntl(ns->socket, F_SETFL, O_NONBLOCK) == -1) { + evdns_log(EVDNS_LOG_WARN, "Error %s (%d) while settings file status flags.", + tor_socket_strerror(errno), errno); + err = 2; + goto out2; + } #endif if (global_bind_addr_is_set && diff --git a/src/or/connection.c b/src/or/connection.c index c659e65..622eadc 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -918,8 +918,11 @@ make_socket_reuseable(tor_socket_t sock) * right after somebody else has let it go. But REUSEADDR on win32 * means you can bind to the port _even when somebody else * already has it bound_. So, don't do that on Win32. */ - setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, (void*) &one, - (socklen_t)sizeof(one)); + if (setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, (void*) &one, + (socklen_t)sizeof(one)) == -1) { + log_warn(LD_NET, "Error setting SO_REUSEADDR flag: %s", + tor_socket_strerror(errno)); + } #endif } @@ -1102,7 +1105,10 @@ connection_listener_new(const struct sockaddr *listensockaddr, tor_assert(0); } - set_socket_nonblocking(s); + if (set_socket_nonblocking(s) == -1) { + tor_close_socket(s); + goto err; + } lis_conn = listener_connection_new(type, listensockaddr->sa_family); conn = TO_CONN(lis_conn); @@ -1265,7 +1271,10 @@ connection_handle_listener_read(connection_t *conn, int new_type) (int)news,(int)conn->s); make_socket_reuseable(news); - set_socket_nonblocking(news); + if (set_socket_nonblocking(news) == -1) { + tor_close_socket(news); + return 0; + } if (options->ConstrainedSockets) set_constrained_socket_buffers(news, (int)options->ConstrainedSockSize); @@ -1494,7 +1503,11 @@ connection_connect(connection_t *conn, const char *address, } } - set_socket_nonblocking(s); + if (set_socket_nonblocking(s) == -1) { + *socket_error = tor_socket_errno(s); + tor_close_socket(s); + return -1; + } if (options->ConstrainedSockets) set_constrained_socket_buffers(s, (int)options->ConstrainedSockSize); diff --git a/src/or/cpuworker.c b/src/or/cpuworker.c index 38c6613..61f9faa 100644 --- a/src/or/cpuworker.c +++ b/src/or/cpuworker.c @@ -535,13 +535,16 @@ spawn_cpuworker(void) conn = connection_new(CONN_TYPE_CPUWORKER, AF_UNIX); - set_socket_nonblocking(fd); - /* set up conn so it's got all the data we need to remember */ conn->s = fd; conn->address = tor_strdup("localhost"); tor_addr_make_unspec(&conn->addr); + if (set_socket_nonblocking(fd) == -1) { + connection_free(conn); /* this closes fd */ + return -1; + } + if (connection_add(conn) < 0) { /* no space, forget it */ log_warn(LD_NET,"connection_add for cpuworker failed. Giving up."); connection_free(conn); /* this closes fd */

1 0

[tor/master] Remove an unused local variable.
by nickm＠torproject.org 18 Mar '13

18 Mar '13

commit 57ffef3f89312be06aea7819cc0a6c6b9dc66452 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Mar 18 14:30:11 2013 -0400 Remove an unused local variable. --- src/ext/eventdns.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/src/ext/eventdns.c b/src/ext/eventdns.c index 0dd7629..66280cc 100644 --- a/src/ext/eventdns.c +++ b/src/ext/eventdns.c @@ -2275,7 +2275,6 @@ _evdns_nameserver_add_impl(const struct sockaddr *address, const struct nameserver *server = server_head, *const started_at = server_head; struct nameserver *ns; - int flags; int err = 0; if (server) {

1 0

[tor/master] Merge remote-tracking branch 'origin/maint-0.2.4'
by nickm＠torproject.org 18 Mar '13

18 Mar '13

commit a770e1cc22e416b84dcc4e8c81fcec5483d2d00c Merge: 3fa9c9b 57ffef3 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Mar 18 14:30:28 2013 -0400 Merge remote-tracking branch 'origin/maint-0.2.4' changes/bug6206 | 6 +++++ src/common/compat.c | 54 ++++++++++++++++++++++++++++++++++++++++++-------- src/common/compat.h | 2 +- src/ext/eventdns.c | 7 +++++- src/or/connection.c | 23 +++++++++++++++++---- src/or/cpuworker.c | 7 ++++- 6 files changed, 81 insertions(+), 18 deletions(-)

1 0