aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--changes/bug87939
-rw-r--r--src/common/address.c4
-rw-r--r--src/common/compat.c4
-rw-r--r--src/common/memarea.c7
-rw-r--r--src/ext/ht.h6
-rw-r--r--src/ext/tinytest.c9
-rw-r--r--src/or/buffers.c2
-rw-r--r--src/or/circuitbuild.c4
-rw-r--r--src/or/circuitlist.c12
-rw-r--r--src/or/circuitmux.c6
-rw-r--r--src/or/circuituse.c19
-rw-r--r--src/or/connection.c2
-rw-r--r--src/or/cpuworker.c2
-rw-r--r--src/or/ext_orport.c2
-rw-r--r--src/or/onion.c6
-rw-r--r--src/or/rendservice.c4
-rw-r--r--src/test/test_addr.c3
-rw-r--r--src/test/test_oom.c4
-rw-r--r--src/test/test_policy.c6
-rw-r--r--src/tools/tor-gencert.c2
20 files changed, 82 insertions, 31 deletions
diff --git a/changes/bug8793 b/changes/bug8793
new file mode 100644
index 000000000..f22c47403
--- /dev/null
+++ b/changes/bug8793
@@ -0,0 +1,9 @@
+ o Minor bugfixes:
+ - Fix numerous warnings from the clang "scan-build" static analyzer.
+ Some of these are programming style issues; some of them are false
+ positives that indicated awkward code; some are undefined behavior
+ cases related to constructing (but not using) invalid pointers;
+ some are assumptions about API behavior; some are using
+ sizeof(ptr) when sizeof(*ptr) would be correct; and one or two are
+ genuine bugs that weren't reachable from the rest of the
+ program. Fixes bug 8793; bugfixes on many, many tor versions.
diff --git a/src/common/address.c b/src/common/address.c
index e5930dedc..2825b123d 100644
--- a/src/common/address.c
+++ b/src/common/address.c
@@ -236,7 +236,9 @@ tor_addr_lookup(const char *name, uint16_t family, tor_addr_t *addr)
hints.ai_family = family;
hints.ai_socktype = SOCK_STREAM;
err = sandbox_getaddrinfo(name, NULL, &hints, &res);
- if (!err) {
+ /* The check for 'res' here shouldn't be necessary, but it makes static
+ * analysis tools happy. */
+ if (!err && res) {
best = NULL;
for (res_p = res; res_p; res_p = res_p->ai_next) {
if (family == AF_UNSPEC) {
diff --git a/src/common/compat.c b/src/common/compat.c
index 9b96a35fe..1ba264a0c 100644
--- a/src/common/compat.c
+++ b/src/common/compat.c
@@ -2198,8 +2198,10 @@ tor_inet_pton(int af, const char *src, void *dst)
else {
unsigned byte1,byte2,byte3,byte4;
char more;
- for (eow = dot-1; eow >= src && TOR_ISDIGIT(*eow); --eow)
+ for (eow = dot-1; eow > src && TOR_ISDIGIT(*eow); --eow)
;
+ if (*eow != ':')
+ return 0;
++eow;
/* We use "scanf" because some platform inet_aton()s are too lax
diff --git a/src/common/memarea.c b/src/common/memarea.c
index e2d07fca9..bcaea0949 100644
--- a/src/common/memarea.c
+++ b/src/common/memarea.c
@@ -291,14 +291,11 @@ memarea_strdup(memarea_t *area, const char *s)
char *
memarea_strndup(memarea_t *area, const char *s, size_t n)
{
- size_t ln;
+ size_t ln = 0;
char *result;
- const char *cp, *end = s+n;
tor_assert(n < SIZE_T_CEILING);
- for (cp = s; cp < end && *cp; ++cp)
+ for (ln = 0; ln < n && s[ln]; ++ln)
;
- /* cp now points to s+n, or to the 0 in the string. */
- ln = cp-s;
result = memarea_alloc(area, ln+1);
memcpy(result, s, ln);
result[ln]='\0';
diff --git a/src/ext/ht.h b/src/ext/ht.h
index e76b4aa4d..4a68673e6 100644
--- a/src/ext/ht.h
+++ b/src/ext/ht.h
@@ -303,14 +303,16 @@ ht_string_hash(const char *s)
#define HT_GENERATE(name, type, field, hashfn, eqfn, load, mallocfn, \
reallocfn, freefn) \
+ /* Primes that aren't too far from powers of two. We stop at */ \
+ /* P=402653189 because P*sizeof(void*) is less than SSIZE_MAX */ \
+ /* even on a 32-bit platform. */ \
static unsigned name##_PRIMES[] = { \
53, 97, 193, 389, \
769, 1543, 3079, 6151, \
12289, 24593, 49157, 98317, \
196613, 393241, 786433, 1572869, \
3145739, 6291469, 12582917, 25165843, \
- 50331653, 100663319, 201326611, 402653189, \
- 805306457, 1610612741 \
+ 50331653, 100663319, 201326611, 402653189 \
}; \
static unsigned name##_N_PRIMES = \
(unsigned)(sizeof(name##_PRIMES)/sizeof(name##_PRIMES[0])); \
diff --git a/src/ext/tinytest.c b/src/ext/tinytest.c
index 3a8e33105..cc054ad34 100644
--- a/src/ext/tinytest.c
+++ b/src/ext/tinytest.c
@@ -478,16 +478,23 @@ tinytest_format_hex_(const void *val_, unsigned long len)
const unsigned char *val = val_;
char *result, *cp;
size_t i;
+ int ellipses = 0;
if (!val)
return strdup("null");
- if (!(result = malloc(len*2+1)))
+ if (len > 1024) {
+ ellipses = 3;
+ len = 1024;
+ }
+ if (!(result = malloc(len*2+4)))
return strdup("<allocation failure>");
cp = result;
for (i=0;i<len;++i) {
*cp++ = "0123456789ABCDEF"[val[i] >> 4];
*cp++ = "0123456789ABCDEF"[val[i] & 0x0f];
}
+ while (ellipses--)
+ *cp++ = '.';
*cp = 0;
return result;
}
diff --git a/src/or/buffers.c b/src/or/buffers.c
index 012ced6d3..fb186081c 100644
--- a/src/or/buffers.c
+++ b/src/or/buffers.c
@@ -322,7 +322,7 @@ buf_shrink_freelists(int free_all)
chunk_t **chp = &freelists[i].head;
chunk_t *chunk;
while (n_to_skip) {
- if (! (*chp)->next) {
+ if (!(*chp) || ! (*chp)->next) {
log_warn(LD_BUG, "I wanted to skip %d chunks in the freelist for "
"%d-byte chunks, but only found %d. (Length %d)",
orig_n_to_skip, (int)freelists[i].alloc_size,
diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index 583ccff78..cd92326b3 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -313,9 +313,9 @@ circuit_rep_hist_note_result(origin_circuit_t *circ)
static int
circuit_cpath_supports_ntor(const origin_circuit_t *circ)
{
- crypt_path_t *head = circ->cpath, *cpath = circ->cpath;
+ crypt_path_t *head, *cpath;
- cpath = head;
+ cpath = head = circ->cpath;
do {
if (cpath->extend_info &&
!tor_mem_is_zero(
diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c
index 8a8fc8b4e..c54a95419 100644
--- a/src/or/circuitlist.c
+++ b/src/or/circuitlist.c
@@ -1269,8 +1269,16 @@ circuit_get_by_rend_token_and_purpose(uint8_t purpose, int is_rend_circ,
circ->base_.marked_for_close)
return NULL;
- if (!circ->rendinfo ||
- ! bool_eq(circ->rendinfo->is_rend_circ, is_rend_circ) ||
+ if (!circ->rendinfo) {
+ char *t = tor_strdup(hex_str(token, REND_TOKEN_LEN));
+ log_warn(LD_BUG, "Wanted a circuit with %s:%d, but lookup returned a "
+ "circuit with no rendinfo set.",
+ safe_str(t), is_rend_circ);
+ tor_free(t);
+ return NULL;
+ }
+
+ if (! bool_eq(circ->rendinfo->is_rend_circ, is_rend_circ) ||
tor_memneq(circ->rendinfo->rend_token, token, REND_TOKEN_LEN)) {
char *t = tor_strdup(hex_str(token, REND_TOKEN_LEN));
log_warn(LD_BUG, "Wanted a circuit with %s:%d, but lookup returned %s:%d",
diff --git a/src/or/circuitmux.c b/src/or/circuitmux.c
index 2d05c748e..52ebfef08 100644
--- a/src/or/circuitmux.c
+++ b/src/or/circuitmux.c
@@ -412,7 +412,11 @@ circuitmux_detach_all_circuits(circuitmux_t *cmux, smartlist_t *detached_out)
i = HT_START(chanid_circid_muxinfo_map, cmux->chanid_circid_map);
while (i) {
to_remove = *i;
- if (to_remove) {
+
+ if (! to_remove) {
+ log_warn(LD_BUG, "Somehow, an HT iterator gave us a NULL pointer.");
+ break;
+ } else {
/* Find a channel and circuit */
chan = channel_find_by_global_id(to_remove->chan_id);
if (chan) {
diff --git a/src/or/circuituse.c b/src/or/circuituse.c
index 75a10ba0c..d10430668 100644
--- a/src/or/circuituse.c
+++ b/src/or/circuituse.c
@@ -537,7 +537,9 @@ circuit_expire_building(void)
"%d guards are live.",
TO_ORIGIN_CIRCUIT(victim)->global_identifier,
circuit_purpose_to_string(victim->purpose),
- TO_ORIGIN_CIRCUIT(victim)->build_state->desired_path_len,
+ TO_ORIGIN_CIRCUIT(victim)->build_state ?
+ TO_ORIGIN_CIRCUIT(victim)->build_state->desired_path_len :
+ -1,
circuit_state_to_string(victim->state),
channel_state_to_string(victim->n_chan->state),
num_live_entry_guards(0));
@@ -561,7 +563,9 @@ circuit_expire_building(void)
"anyway. %d guards are live.",
TO_ORIGIN_CIRCUIT(victim)->global_identifier,
circuit_purpose_to_string(victim->purpose),
- TO_ORIGIN_CIRCUIT(victim)->build_state->desired_path_len,
+ TO_ORIGIN_CIRCUIT(victim)->build_state ?
+ TO_ORIGIN_CIRCUIT(victim)->build_state->desired_path_len :
+ -1,
circuit_state_to_string(victim->state),
channel_state_to_string(victim->n_chan->state),
(long)build_close_ms,
@@ -707,7 +711,8 @@ circuit_expire_building(void)
* and we have tried to send an INTRODUCE1 cell specifying it.
* Thus, if the pending_final_cpath field *is* NULL, then we
* want to not spare it. */
- if (TO_ORIGIN_CIRCUIT(victim)->build_state->pending_final_cpath ==
+ if (TO_ORIGIN_CIRCUIT(victim)->build_state &&
+ TO_ORIGIN_CIRCUIT(victim)->build_state->pending_final_cpath ==
NULL)
break;
/* fallthrough! */
@@ -753,7 +758,9 @@ circuit_expire_building(void)
TO_ORIGIN_CIRCUIT(victim)->has_opened,
victim->state, circuit_state_to_string(victim->state),
victim->purpose,
- TO_ORIGIN_CIRCUIT(victim)->build_state->desired_path_len);
+ TO_ORIGIN_CIRCUIT(victim)->build_state ?
+ TO_ORIGIN_CIRCUIT(victim)->build_state->desired_path_len :
+ -1);
else
log_info(LD_CIRC,
"Abandoning circ %u %u (state %d,%d:%s, purpose %d, len %d)",
@@ -762,7 +769,9 @@ circuit_expire_building(void)
TO_ORIGIN_CIRCUIT(victim)->has_opened,
victim->state,
circuit_state_to_string(victim->state), victim->purpose,
- TO_ORIGIN_CIRCUIT(victim)->build_state->desired_path_len);
+ TO_ORIGIN_CIRCUIT(victim)->build_state ?
+ TO_ORIGIN_CIRCUIT(victim)->build_state->desired_path_len :
+ -1);
circuit_log_path(LOG_INFO,LD_CIRC,TO_ORIGIN_CIRCUIT(victim));
if (victim->purpose == CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT)
diff --git a/src/or/connection.c b/src/or/connection.c
index 9614fa52d..3cc4e09fb 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -4812,6 +4812,8 @@ get_proxy_addrport(tor_addr_t *addr, uint16_t *port, int *proxy_type,
}
}
+ tor_addr_make_unspec(addr);
+ *port = 0;
*proxy_type = PROXY_NONE;
return 0;
}
diff --git a/src/or/cpuworker.c b/src/or/cpuworker.c
index 209274da6..6b6a68afe 100644
--- a/src/or/cpuworker.c
+++ b/src/or/cpuworker.c
@@ -436,7 +436,7 @@ cpuworker_main(void *data)
if (req.task == CPUWORKER_TASK_ONION) {
const create_cell_t *cc = &req.create_cell;
created_cell_t *cell_out = &rpl.created_cell;
- struct timeval tv_start, tv_end;
+ struct timeval tv_start = {0,0}, tv_end;
int n;
rpl.timed = req.timed;
rpl.started_at = req.started_at;
diff --git a/src/or/ext_orport.c b/src/or/ext_orport.c
index d5a0fa1ee..0d28a9199 100644
--- a/src/or/ext_orport.c
+++ b/src/or/ext_orport.c
@@ -256,7 +256,7 @@ handle_client_auth_nonce(const char *client_nonce, size_t client_nonce_len,
base16_encode(server_nonce_encoded, sizeof(server_nonce_encoded),
server_nonce, sizeof(server_nonce));
base16_encode(client_nonce_encoded, sizeof(client_nonce_encoded),
- client_nonce, sizeof(client_nonce));
+ client_nonce, EXT_OR_PORT_AUTH_NONCE_LEN);
log_debug(LD_GENERAL,
"server_hash: '%s'\nserver_nonce: '%s'\nclient_nonce: '%s'",
diff --git a/src/or/onion.c b/src/or/onion.c
index 30b983d91..72571b7bd 100644
--- a/src/or/onion.c
+++ b/src/or/onion.c
@@ -329,12 +329,14 @@ onion_queue_entry_remove(onion_queue_t *victim)
void
clear_pending_onions(void)
{
- onion_queue_t *victim;
+ onion_queue_t *victim, *next;
int i;
for (i=0; i<=MAX_ONION_HANDSHAKE_TYPE; i++) {
- while ((victim = TOR_TAILQ_FIRST(&ol_list[i]))) {
+ for (victim = TOR_TAILQ_FIRST(&ol_list[i]); victim; victim = next) {
+ next = TOR_TAILQ_NEXT(victim,next);
onion_queue_entry_remove(victim);
}
+ tor_assert(TOR_TAILQ_EMPTY(&ol_list[i]));
}
memset(ol_entries, 0, sizeof(ol_entries));
}
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index abd78da73..5a81d0785 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -2041,7 +2041,7 @@ rend_service_decrypt_intro(
if (err_msg_out && !err_msg) {
tor_asprintf(&err_msg,
"unknown INTRODUCE%d error decrypting encrypted part",
- (int)(intro->type));
+ intro ? (int)(intro->type) : -1);
}
if (status >= 0) status = -1;
@@ -2147,7 +2147,7 @@ rend_service_parse_intro_plaintext(
if (err_msg_out && !err_msg) {
tor_asprintf(&err_msg,
"unknown INTRODUCE%d error parsing encrypted part",
- (int)(intro->type));
+ intro ? (int)(intro->type) : -1);
}
if (status >= 0) status = -1;
diff --git a/src/test/test_addr.c b/src/test/test_addr.c
index cee2dcf2a..50011e606 100644
--- a/src/test/test_addr.c
+++ b/src/test/test_addr.c
@@ -346,6 +346,9 @@ test_addr_ip6_helpers(void)
test_pton6_bad("a:::b:c");
test_pton6_bad(":::a:b:c");
test_pton6_bad("a:b:c:::");
+ test_pton6_bad("1.2.3.4");
+ test_pton6_bad(":1.2.3.4");
+ test_pton6_bad(".2.3.4");
/* test internal checking */
test_external_ip("fbff:ffff::2:7", 0);
diff --git a/src/test/test_oom.c b/src/test/test_oom.c
index cc6e53235..989ca1203 100644
--- a/src/test/test_oom.c
+++ b/src/test/test_oom.c
@@ -82,8 +82,8 @@ add_bytes_to_buf(generic_buffer_t *buf, size_t n_bytes)
char b[3000];
while (n_bytes) {
- size_t this_add = n_bytes > sizeof(buf) ? sizeof(buf) : n_bytes;
- crypto_rand(b, sizeof(b));
+ size_t this_add = n_bytes > sizeof(b) ? sizeof(b) : n_bytes;
+ crypto_rand(b, this_add);
generic_buffer_add(buf, b, this_add);
n_bytes -= this_add;
}
diff --git a/src/test/test_policy.c b/src/test/test_policy.c
index d2ba1612d..491c9a21f 100644
--- a/src/test/test_policy.c
+++ b/src/test/test_policy.c
@@ -417,8 +417,10 @@ test_dump_exit_policy_to_string(void *arg)
done:
- SMARTLIST_FOREACH(ri->exit_policy, addr_policy_t *,
- entry, addr_policy_free(entry));
+ if (ri->exit_policy) {
+ SMARTLIST_FOREACH(ri->exit_policy, addr_policy_t *,
+ entry, addr_policy_free(entry));
+ }
tor_free(ri);
tor_free(ep);
}
diff --git a/src/tools/tor-gencert.c b/src/tools/tor-gencert.c
index d0c30b8b0..e799df5ca 100644
--- a/src/tools/tor-gencert.c
+++ b/src/tools/tor-gencert.c
@@ -302,6 +302,7 @@ load_identity_key(void)
if (!identity_key) {
log_err(LD_GENERAL, "Couldn't read identity key from %s",
identity_key_file);
+ fclose(f);
return 1;
}
fclose(f);
@@ -322,6 +323,7 @@ load_signing_key(void)
}
if (!(signing_key = PEM_read_PrivateKey(f, NULL, NULL, NULL))) {
log_err(LD_GENERAL, "Couldn't read siging key from %s", signing_key_file);
+ fclose(f);
return 1;
}
fclose(f);
generated by cgit v1.2.3 (git 2.25.4) at 2024-05-20 09:10:45 +0000