From 4ff170d7b1cbe4074cb85271b82a8963eccc8286 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 12 Sep 2017 21:09:18 -0400 Subject: [PATCH] Fix warnings about passing uninitialized buffers into functions Most of these buffers were never actually inspected, but it's still bad style. --- src/common/buffers.c | 2 ++ src/common/compat.c | 1 + src/or/config.c | 2 ++ src/or/dirvote.c | 17 +++++++++-------- src/or/parsecommon.c | 1 + src/test/bench.c | 1 + src/test/test_crypto.c | 2 ++ src/test/test_dir.c | 2 +- src/test/test_hs_intropoint.c | 4 +++- src/test/test_shared_random.c | 2 ++ src/tools/tor-gencert.c | 12 ++++++++---- 11 files changed, 32 insertions(+), 14 deletions(-) diff --git a/src/common/buffers.c b/src/common/buffers.c index 50673646df..9583c70361 100644 --- a/src/common/buffers.c +++ b/src/common/buffers.c @@ -907,6 +907,8 @@ buf_peek_startswith(const buf_t *buf, const char *cmd) { char tmp[PEEK_BUF_STARTSWITH_MAX]; size_t clen = strlen(cmd); + if (clen == 0) + return 1; if (BUG(clen > sizeof(tmp))) return 0; if (buf->datalen < clen) diff --git a/src/common/compat.c b/src/common/compat.c index 68938ae91f..ab117f7c72 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -2580,6 +2580,7 @@ tor_inet_pton(int af, const char *src, void *dst) int gapPos = -1, i, setWords=0; const char *dot = strchr(src, '.'); const char *eow; /* end of words. */ + memset(words, 0xf8, sizeof(words)); if (dot == src) return 0; else if (!dot) diff --git a/src/or/config.c b/src/or/config.c index a5bda8be00..76461d75bf 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -6072,6 +6072,8 @@ parse_dir_authority_line(const char *line, dirinfo_type_t required_type, dirinfo_type_t type = 0; double weight = 1.0; + memset(v3_digest, 0, sizeof(v3_digest)); + items = smartlist_new(); smartlist_split_string(items, line, NULL, SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, -1); diff --git a/src/or/dirvote.c b/src/or/dirvote.c index 5dd2179521..ba0ab7a776 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -3993,14 +3993,15 @@ dirvote_format_all_microdesc_vote_lines(const routerinfo_t *ri, time_t now, while ((ep = entries)) { char buf[128]; vote_microdesc_hash_t *h; - dirvote_format_microdesc_vote_line(buf, sizeof(buf), ep->md, - ep->low, ep->high); - h = tor_malloc_zero(sizeof(vote_microdesc_hash_t)); - h->microdesc_hash_line = tor_strdup(buf); - h->next = result; - result = h; - ep->md->last_listed = now; - smartlist_add(microdescriptors_out, ep->md); + if (dirvote_format_microdesc_vote_line(buf, sizeof(buf), ep->md, + ep->low, ep->high) >= 0) { + h = tor_malloc_zero(sizeof(vote_microdesc_hash_t)); + h->microdesc_hash_line = tor_strdup(buf); + h->next = result; + result = h; + ep->md->last_listed = now; + smartlist_add(microdescriptors_out, ep->md); + } entries = ep->next; tor_free(ep); } diff --git a/src/or/parsecommon.c b/src/or/parsecommon.c index 6b5359303a..6c3dd3100e 100644 --- a/src/or/parsecommon.c +++ b/src/or/parsecommon.c @@ -161,6 +161,7 @@ get_token_arguments(memarea_t *area, directory_token_t *tok, char *cp = mem; int j = 0; char *args[MAX_ARGS]; + memset(args, 0, sizeof(args)); while (*cp) { if (j == MAX_ARGS) return -1; diff --git a/src/test/bench.c b/src/test/bench.c index a44dc94a61..9d589332ae 100644 --- a/src/test/bench.c +++ b/src/test/bench.c @@ -200,6 +200,7 @@ bench_onion_ntor_impl(void) curve25519_public_key_generate(&keypair2.pubkey, &keypair2.seckey); dimap_add_entry(&keymap, keypair1.pubkey.public_key, &keypair1); dimap_add_entry(&keymap, keypair2.pubkey.public_key, &keypair2); + crypto_rand((char *)nodeid, sizeof(nodeid)); reset_perftime(); start = perftime(); diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c index c540aaed6c..5d079e9f30 100644 --- a/src/test/test_crypto.c +++ b/src/test/test_crypto.c @@ -2595,6 +2595,8 @@ test_crypto_ed25519_testvectors(void *arg) ed25519_signature_t sig; int sign; + memset(&curvekp, 0xd0, sizeof(curvekp)); + #define DECODE(p,s) base16_decode((char*)(p),sizeof(p),(s),strlen(s)) #define EQ(a,h) test_memeq_hex((const char*)(a), (h)) diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 896e16ce05..3d1fb00dba 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -3370,7 +3370,7 @@ mock_get_options(void) static void reset_routerstatus(routerstatus_t *rs, const char *hex_identity_digest, - int32_t ipv4_addr) + uint32_t ipv4_addr) { memset(rs, 0, sizeof(routerstatus_t)); base16_decode(rs->identity_digest, sizeof(rs->identity_digest), diff --git a/src/test/test_hs_intropoint.c b/src/test/test_hs_intropoint.c index 2cce8a3703..1e570630c0 100644 --- a/src/test/test_hs_intropoint.c +++ b/src/test/test_hs_intropoint.c @@ -783,7 +783,7 @@ static void test_received_introduce1_handling(void *arg) { int ret; - uint8_t *request = NULL, buf[128]; + uint8_t *request = NULL, buf[128];; trn_cell_introduce1_t *cell = NULL; or_circuit_t *circ = NULL; @@ -796,6 +796,7 @@ test_received_introduce1_handling(void *arg) /* Too small request length. An INTRODUCE1 expect at the very least a * DIGEST_LEN size. */ { + memset(buf, 0, sizeof(buf)); circ = helper_create_intro_circuit(); ret = hs_intro_received_introduce1(circ, buf, DIGEST_LEN - 1); tt_int_op(ret, OP_EQ, -1); @@ -809,6 +810,7 @@ test_received_introduce1_handling(void *arg) { circ = helper_create_intro_circuit(); uint8_t test[2]; /* Too small request. */ + memset(test, 0, sizeof(test)); ret = handle_introduce1(circ, test, sizeof(test)); tor_free(circ->p_chan); circuit_free(TO_CIRCUIT(circ)); diff --git a/src/test/test_shared_random.c b/src/test/test_shared_random.c index 4c303cbb35..a9d58e6b8b 100644 --- a/src/test/test_shared_random.c +++ b/src/test/test_shared_random.c @@ -1231,6 +1231,8 @@ test_keep_commit(void *arg) state = get_sr_state(); } + crypto_rand((char*)fp, sizeof(fp)); + /* Test this very important function that tells us if we should keep a * commit or not in our state. Most of it depends on the phase and what's * in the commit so we'll change the commit as we go. */ diff --git a/src/tools/tor-gencert.c b/src/tools/tor-gencert.c index 395535697f..600e2252d4 100644 --- a/src/tools/tor-gencert.c +++ b/src/tools/tor-gencert.c @@ -430,7 +430,7 @@ key_to_string(EVP_PKEY *key) static int get_fingerprint(EVP_PKEY *pkey, char *out) { - int r = 1; + int r = -1; crypto_pk_t *pk = crypto_new_pk_from_rsa_(EVP_PKEY_get1_RSA(pkey)); if (pk) { r = crypto_pk_get_fingerprint(pk, out, 0); @@ -443,7 +443,7 @@ get_fingerprint(EVP_PKEY *pkey, char *out) static int get_digest(EVP_PKEY *pkey, char *out) { - int r = 1; + int r = -1; crypto_pk_t *pk = crypto_new_pk_from_rsa_(EVP_PKEY_get1_RSA(pkey)); if (pk) { r = crypto_pk_get_digest(pk, out); @@ -472,8 +472,12 @@ generate_certificate(void) char signature[1024]; /* handles up to 8192-bit keys. */ int r; - get_fingerprint(identity_key, fingerprint); - get_digest(identity_key, id_digest); + if (get_fingerprint(identity_key, fingerprint) < 0) { + return -1; + } + if (get_digest(identity_key, id_digest)) { + return -1; + } tor_localtime_r(&now, &tm); tm.tm_mon += months_lifetime;