From 5b9508c9a5d757223c62749c51eaf7453ff26691 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 3 Aug 2020 10:11:03 -0400 Subject: Add a tor_str_wipe_and_free() function. Frequently we want to do if (s) { memwipe(s, 0, sizeof(s)); tor_free(s); } and it's good to have a way to do this concisely. --- src/lib/crypt_ops/crypto_util.c | 14 ++++++++++++++ src/lib/crypt_ops/crypto_util.h | 10 ++++++++++ 2 files changed, 24 insertions(+) diff --git a/src/lib/crypt_ops/crypto_util.c b/src/lib/crypt_ops/crypto_util.c index 67a1a9eb92..2f821fa024 100644 --- a/src/lib/crypt_ops/crypto_util.c +++ b/src/lib/crypt_ops/crypto_util.c @@ -109,3 +109,17 @@ memwipe(void *mem, uint8_t byte, size_t sz) **/ memset(mem, byte, sz); } + +/** + * Securely all memory in str, then free it. + * + * As tor_free(), tolerates null pointers. + **/ +void +tor_str_wipe_and_free_(char *str) +{ + if (!str) + return; + memwipe(str, 0, strlen(str)); + tor_free_(str); +} diff --git a/src/lib/crypt_ops/crypto_util.h b/src/lib/crypt_ops/crypto_util.h index 613a1bd0dd..b5d7f62521 100644 --- a/src/lib/crypt_ops/crypto_util.h +++ b/src/lib/crypt_ops/crypto_util.h @@ -14,8 +14,18 @@ #define TOR_CRYPTO_UTIL_H #include "lib/cc/torint.h" +#include "lib/malloc/malloc.h" /** OpenSSL-based utility functions. */ void memwipe(void *mem, uint8_t byte, size_t sz); +void tor_str_wipe_and_free_(char *str); +/** + * Securely all memory in str, then free it. + * + * As tor_free(), tolerates null pointers, and sets str to NULL. + **/ +#define tor_str_wipe_and_free(str) \ + FREE_AND_NULL(char, tor_str_wipe_and_free_, (str)) + #endif /* !defined(TOR_CRYPTO_UTIL_H) */ -- cgit v1.2.3-54-g00ecf From ea876ab00e223b0c1ba022cc27861cfbbde31b64 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 3 Aug 2020 10:15:08 -0400 Subject: Wipe address strings from connections before freeing them them. This is a defense-in-depth fix; closes 6198. --- changes/ticket6198 | 3 +++ src/core/mainloop/connection.c | 8 ++++---- 2 files changed, 7 insertions(+), 4 deletions(-) create mode 100644 changes/ticket6198 diff --git a/changes/ticket6198 b/changes/ticket6198 new file mode 100644 index 0000000000..7f3fdf2fa7 --- /dev/null +++ b/changes/ticket6198 @@ -0,0 +1,3 @@ + o Minor features (defense in depth): + - Wipe more data from connection address fields before returning them to + the memory heap. Closes ticket 6198. diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index 3595bba85c..4b321fe72f 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -637,7 +637,7 @@ connection_free_minimal(connection_t *conn) } } - tor_free(conn->address); + tor_str_wipe_and_free(conn->address); if (connection_speaks_cells(conn)) { or_connection_t *or_conn = TO_OR_CONN(conn); @@ -657,7 +657,7 @@ connection_free_minimal(connection_t *conn) } or_handshake_state_free(or_conn->handshake_state); or_conn->handshake_state = NULL; - tor_free(or_conn->nickname); + tor_str_wipe_and_free(or_conn->nickname); if (or_conn->chan) { /* Owww, this shouldn't happen, but... */ channel_t *base_chan = TLS_CHAN_TO_BASE(or_conn->chan); @@ -677,8 +677,8 @@ connection_free_minimal(connection_t *conn) } if (conn->type == CONN_TYPE_AP) { entry_connection_t *entry_conn = TO_ENTRY_CONN(conn); - tor_free(entry_conn->chosen_exit_name); - tor_free(entry_conn->original_dest_address); + tor_str_wipe_and_free(entry_conn->chosen_exit_name); + tor_str_wipe_and_free(entry_conn->original_dest_address); if (entry_conn->socks_request) socks_request_free(entry_conn->socks_request); if (entry_conn->pending_optimistic_data) { -- cgit v1.2.3-54-g00ecf