[tor-commits] [tor/master] Fix remaining cases of using consensus without a len parameter.
nickm at torproject.org
nickm at torproject.org
Wed Oct 31 14:13:23 UTC 2018
commit 594140574e7366efac693d440a636a1e1cce82ff
Author: Nick Mathewson <nickm at 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)
{
More information about the tor-commits
mailing list