From 4b571d3ab3d4c8e13fe0bc73f15431294d19615a Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 28 Dec 2012 22:38:42 -0500 Subject: [PATCH 1/3] Fix memory leak in safe-cookie authentication code Coverity spotted this. Bug 7816. Fix on 0.2.3.13-alpha. --- changes/bug7816_023 | 3 +++ src/or/control.c | 2 ++ 2 files changed, 5 insertions(+) create mode 100644 changes/bug7816_023 diff --git a/changes/bug7816_023 b/changes/bug7816_023 new file mode 100644 index 0000000000..cfa754a996 --- /dev/null +++ b/changes/bug7816_023 @@ -0,0 +1,3 @@ + o Minor bugfixes (memory leak, controller): + - Fix a memory leak during safe-cookie controller authentication. + Spotted by Coverity. Fixes part of bug 7816; bugfix on 0.2.3.13-alpha. diff --git a/src/or/control.c b/src/or/control.c index 913d18a7fc..fc7bae23e1 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -3099,6 +3099,8 @@ handle_control_authchallenge(control_connection_t *conn, uint32_t len, "SERVERNONCE=%s\r\n", server_hash_encoded, server_nonce_encoded); + + tor_free(client_nonce); return 0; } From b509ead20d93277697c1cb961a2ae520bce7fd7f Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 28 Dec 2012 22:44:45 -0500 Subject: [PATCH 2/3] Avoid leaking headers received from SSL proxy Fixes part of 7816. Spotted by coverity. Fix on 0.2.2.1-alpha. --- changes/bug7816_023 | 4 ++++ src/or/connection.c | 1 + 2 files changed, 5 insertions(+) diff --git a/changes/bug7816_023 b/changes/bug7816_023 index cfa754a996..a4530292cc 100644 --- a/changes/bug7816_023 +++ b/changes/bug7816_023 @@ -1,3 +1,7 @@ o Minor bugfixes (memory leak, controller): - Fix a memory leak during safe-cookie controller authentication. Spotted by Coverity. Fixes part of bug 7816; bugfix on 0.2.3.13-alpha. + + o Minor bugfixes (memory leak, HTTPS proxy support): + - Fix a memory leak when receiving headers from an HTTPS proxy. + Spotted by Coverity. Fixes part of bug 7816; bugfix on 0.2.1.1-alpha. diff --git a/src/or/connection.c b/src/or/connection.c index eac9c4f32b..4c6826269d 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -1607,6 +1607,7 @@ connection_read_https_proxy_response(connection_t *conn) tor_free(headers); return -1; } + tor_free(headers); if (!reason) reason = tor_strdup("[no reason given]"); if (status_code == 200) { From d3aabf4db176a44d19046b58b99f2edb8c5f49bb Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 28 Dec 2012 22:49:32 -0500 Subject: [PATCH 3/3] Fix various small leaks on error cases Spotted by coverity, bug 7816, bugfix on various versions. --- changes/bug7816_023_small | 3 +++ src/common/log.c | 4 +++- src/common/util.c | 4 +++- src/or/connection.c | 3 +++ src/or/geoip.c | 5 ++++- 5 files changed, 16 insertions(+), 3 deletions(-) create mode 100644 changes/bug7816_023_small diff --git a/changes/bug7816_023_small b/changes/bug7816_023_small new file mode 100644 index 0000000000..cd90f035f1 --- /dev/null +++ b/changes/bug7816_023_small @@ -0,0 +1,3 @@ + o Minor bugfixes: + - Fix various places where we leak file descriptors or memory on + error cases. Spotted by coverity. Fixes parts of bug 7816. diff --git a/src/common/log.c b/src/common/log.c index 5e2e6b5b50..5f0b4f8d9c 100644 --- a/src/common/log.c +++ b/src/common/log.c @@ -833,8 +833,10 @@ add_file_log(const log_severity_list_t *severity, const char *filename) fd = tor_open_cloexec(filename, O_WRONLY|O_CREAT|O_APPEND, 0644); if (fd<0) return -1; - if (tor_fd_seekend(fd)<0) + if (tor_fd_seekend(fd)<0) { + close(fd); return -1; + } LOCK_LOGS(); add_stream_log_impl(severity, filename, fd); diff --git a/src/common/util.c b/src/common/util.c index 6fb597a3a5..29aa83e5d6 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -2305,8 +2305,10 @@ read_file_to_str(const char *filename, int flags, struct stat *stat_out) return NULL; } - if ((uint64_t)(statbuf.st_size)+1 >= SIZE_T_CEILING) + if ((uint64_t)(statbuf.st_size)+1 >= SIZE_T_CEILING) { + close(fd); return NULL; + } string = tor_malloc((size_t)(statbuf.st_size+1)); diff --git a/src/or/connection.c b/src/or/connection.c index 4c6826269d..8c4228e5cc 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -992,6 +992,7 @@ connection_listener_new(const struct sockaddr *listensockaddr, if (bind(s, listensockaddr, (socklen_t)sizeof(struct sockaddr_un)) == -1) { log_warn(LD_NET,"Bind to %s failed: %s.", address, tor_socket_strerror(tor_socket_errno(s))); + tor_close_socket(s); goto err; } #ifdef HAVE_PWD_H @@ -1000,9 +1001,11 @@ connection_listener_new(const struct sockaddr *listensockaddr, if (pw == NULL) { log_warn(LD_NET,"Unable to chown() %s socket: user %s not found.", address, options->User); + tor_close_socket(s); } else if (chown(address, pw->pw_uid, pw->pw_gid) < 0) { log_warn(LD_NET,"Unable to chown() %s socket: %s.", address, strerror(errno)); + tor_close_socket(s); goto err; } } diff --git a/src/or/geoip.c b/src/or/geoip.c index 6b7cc82b82..8ca95af2e6 100644 --- a/src/or/geoip.c +++ b/src/or/geoip.c @@ -1316,8 +1316,11 @@ load_bridge_stats(time_t now) fname = get_datadir_fname2("stats", "bridge-stats"); contents = read_file_to_str(fname, RFTS_IGNORE_MISSING, NULL); - if (contents && validate_bridge_stats(contents, now)) + if (contents && validate_bridge_stats(contents, now)) { bridge_stats_extrainfo = contents; + } else { + tor_free(contents); + } tor_free(fname); }