commit 594140574e7366efac693d440a636a1e1cce82ff Author: Nick Mathewson <nickm@torproject.org> Date: Wed Oct 24 11:06:34 2018 -0400 Fix remaining cases of using consensus without a len parameter. (Thanks to cyberpunks for noting two of them!) --- src/feature/dircache/consdiffmgr.c | 28 ++++++++++++++++++++++++---- src/feature/dircache/consdiffmgr.h | 6 ++++++ src/feature/dircache/dirserv.c | 5 ++++- src/feature/dircache/dirserv.h | 1 + src/feature/nodelist/networkstatus.c | 5 +++-- src/test/test_consdiffmgr.c | 2 ++ src/test/test_dir_handle_get.c | 2 ++ 7 files changed, 42 insertions(+), 7 deletions(-) diff --git a/src/feature/dircache/consdiffmgr.c b/src/feature/dircache/consdiffmgr.c index bf3a0ef3c..2cddf68b6 100644 --- a/src/feature/dircache/consdiffmgr.c +++ b/src/feature/dircache/consdiffmgr.c @@ -189,6 +189,7 @@ static consdiff_cfg_t consdiff_cfg = { static int consdiffmgr_ensure_space_for_files(int n); static int consensus_queue_compression_work(const char *consensus, + size_t consensus_len, const networkstatus_t *as_parsed); static int consensus_diff_queue_diff_work(consensus_cache_entry_t *diff_from, consensus_cache_entry_t *diff_to); @@ -509,8 +510,25 @@ get_max_age_to_cache(void) MAX_MAX_AGE_TO_CACHE); } +#ifdef TOR_UNIT_TESTS +/** As consdiffmgr_add_consensus, but requires a nul-terminated input. For + * testing. */ +int +consdiffmgr_add_consensus_nulterm(const char *consensus, + const networkstatus_t *as_parsed) +{ + size_t len = strlen(consensus); + /* make a non-nul-terminated copy so that we can have a better chance + * of catching errors. */ + char *ctmp = tor_memdup(consensus, len); + int r = consdiffmgr_add_consensus(ctmp, len, as_parsed); + tor_free(ctmp); + return r; +} +#endif + /** - * Given a string containing a networkstatus consensus, and the results of + * Given a buffer containing a networkstatus consensus, and the results of * having parsed that consensus, add that consensus to the cache if it is not * already present and not too old. Create new consensus diffs from or to * that consensus as appropriate. @@ -519,6 +537,7 @@ get_max_age_to_cache(void) */ int consdiffmgr_add_consensus(const char *consensus, + size_t consensus_len, const networkstatus_t *as_parsed) { if (BUG(consensus == NULL) || BUG(as_parsed == NULL)) @@ -544,7 +563,7 @@ consdiffmgr_add_consensus(const char *consensus, } /* We don't have it. Add it to the cache. */ - return consensus_queue_compression_work(consensus, as_parsed); + return consensus_queue_compression_work(consensus, consensus_len, as_parsed); } /** @@ -1826,14 +1845,15 @@ static int background_compression = 0; */ static int consensus_queue_compression_work(const char *consensus, + size_t consensus_len, const networkstatus_t *as_parsed) { tor_assert(consensus); tor_assert(as_parsed); consensus_compress_worker_job_t *job = tor_malloc_zero(sizeof(*job)); - job->consensus = tor_strdup(consensus); - job->consensus_len = strlen(consensus); + job->consensus = tor_memdup_nulterm(consensus, consensus_len); + job->consensus_len = strlen(job->consensus); job->flavor = as_parsed->flavor; char va_str[ISO_TIME_LEN+1]; diff --git a/src/feature/dircache/consdiffmgr.h b/src/feature/dircache/consdiffmgr.h index d6f273cc4..011c8799d 100644 --- a/src/feature/dircache/consdiffmgr.h +++ b/src/feature/dircache/consdiffmgr.h @@ -22,6 +22,7 @@ typedef struct consdiff_cfg_t { struct consensus_cache_entry_t; // from conscache.h int consdiffmgr_add_consensus(const char *consensus, + size_t consensus_len, const networkstatus_t *as_parsed); consdiff_status_t consdiffmgr_find_consensus( @@ -73,4 +74,9 @@ STATIC int uncompress_or_set_ptr(const char **out, size_t *outlen, consensus_cache_entry_t *ent); #endif /* defined(CONSDIFFMGR_PRIVATE) */ +#ifdef TOR_UNIT_TESTS +int consdiffmgr_add_consensus_nulterm(const char *consensus, + const networkstatus_t *as_parsed); +#endif + #endif /* !defined(TOR_CONSDIFFMGR_H) */ diff --git a/src/feature/dircache/dirserv.c b/src/feature/dircache/dirserv.c index b85db8324..433d3f4ce 100644 --- a/src/feature/dircache/dirserv.c +++ b/src/feature/dircache/dirserv.c @@ -1272,6 +1272,7 @@ free_cached_dir_(void *_d) * validation is performed. */ void dirserv_set_cached_consensus_networkstatus(const char *networkstatus, + size_t networkstatus_len, const char *flavor_name, const common_digests_t *digests, const uint8_t *sha3_as_signed, @@ -1282,7 +1283,9 @@ dirserv_set_cached_consensus_networkstatus(const char *networkstatus, if (!cached_consensuses) cached_consensuses = strmap_new(); - new_networkstatus = new_cached_dir(tor_strdup(networkstatus), published); + new_networkstatus = + new_cached_dir(tor_memdup_nulterm(networkstatus, networkstatus_len), + published); memcpy(&new_networkstatus->digests, digests, sizeof(common_digests_t)); memcpy(&new_networkstatus->digest_sha3_as_signed, sha3_as_signed, DIGEST256_LEN); diff --git a/src/feature/dircache/dirserv.h b/src/feature/dircache/dirserv.h index 9be4bf9db..c430987aa 100644 --- a/src/feature/dircache/dirserv.h +++ b/src/feature/dircache/dirserv.h @@ -148,6 +148,7 @@ int directory_too_idle_to_fetch_descriptors(const or_options_t *options, cached_dir_t *dirserv_get_consensus(const char *flavor_name); void dirserv_set_cached_consensus_networkstatus(const char *consensus, + size_t consensus_len, const char *flavor_name, const common_digests_t *digests, const uint8_t *sha3_as_signed, diff --git a/src/feature/nodelist/networkstatus.c b/src/feature/nodelist/networkstatus.c index 847ca0cdf..97a8779be 100644 --- a/src/feature/nodelist/networkstatus.c +++ b/src/feature/nodelist/networkstatus.c @@ -2107,17 +2107,18 @@ networkstatus_set_current_consensus(const char *consensus, if (we_want_to_fetch_flavor(options, flav)) { dirserv_set_cached_consensus_networkstatus(consensus, + consensus_len, flavor, &c->digests, c->digest_sha3_as_signed, c->valid_after); if (dir_server_mode(get_options())) { - consdiffmgr_add_consensus(consensus, c); + consdiffmgr_add_consensus(consensus, consensus_len, c); } } if (!from_cache) { - write_str_to_file(consensus_fname, consensus, 0); + write_bytes_to_file(consensus_fname, consensus, consensus_len, 0); } warn_early_consensus(c, flavor, now); diff --git a/src/test/test_consdiffmgr.c b/src/test/test_consdiffmgr.c index 4b49fdb6a..966314be1 100644 --- a/src/test/test_consdiffmgr.c +++ b/src/test/test_consdiffmgr.c @@ -21,6 +21,8 @@ #include "test/test.h" #include "test/log_test_helpers.h" +#define consdiffmgr_add_consensus consdiffmgr_add_consensus_nulterm + static char * consensus_diff_apply_(const char *c, const char *d) { diff --git a/src/test/test_dir_handle_get.c b/src/test/test_dir_handle_get.c index ecb7c1b5f..b59139691 100644 --- a/src/test/test_dir_handle_get.c +++ b/src/test/test_dir_handle_get.c @@ -67,6 +67,8 @@ ENABLE_GCC_WARNING(overlength-strings) #define NOT_ENOUGH_CONSENSUS_SIGNATURES "HTTP/1.0 404 " \ "Consensus not signed by sufficient number of requested authorities\r\n\r\n" +#define consdiffmgr_add_consensus consdiffmgr_add_consensus_nulterm + static dir_connection_t * new_dir_conn(void) {