Browse Source

Merge branch 'scan-build-032'

Nick Mathewson 6 years ago
parent
commit
76c33f7ff4

+ 43 - 12
scripts/test/scan-build.sh

@@ -5,37 +5,68 @@
 # This script is used for running a bunch of clang scan-build checkers
 # on Tor.
 
+# These don't seem to cause false positives in our code, so let's turn
+# them on.
 CHECKERS="\
-    -disable-checker deadcode.DeadStores \
-    -enable-checker alpha.core.CastSize \
+    -enable-checker alpha.core.CallAndMessageUnInitRefArg \
     -enable-checker alpha.core.CastToStruct \
+    -enable-checker alpha.core.Conversion \
+    -enable-checker alpha.core.FixedAddr \
     -enable-checker alpha.core.IdenticalExpr \
+    -enable-checker alpha.core.PointerArithm \
     -enable-checker alpha.core.SizeofPtr \
-    -enable-checker alpha.security.ArrayBoundV2 \
+    -enable-checker alpha.core.TestAfterDivZero \
     -enable-checker alpha.security.MallocOverflow \
     -enable-checker alpha.security.ReturnPtrRange \
-    -enable-checker alpha.unix.SimpleStream
+    -enable-checker alpha.unix.BlockInCriticalSection \
+    -enable-checker alpha.unix.Chroot \
+    -enable-checker alpha.unix.PthreadLock \
+    -enable-checker alpha.unix.PthreadLock \
+    -enable-checker alpha.unix.SimpleStream \
+    -enable-checker alpha.unix.Stream \
     -enable-checker alpha.unix.cstring.BufferOverlap \
     -enable-checker alpha.unix.cstring.NotNullTerminated \
-    -enable-checker alpha.unix.cstring.OutOfBounds \
-    -enable-checker alpha.core.FixedAddr \
+    -enable-checker alpha.valist.CopyToSelf \
+    -enable-checker alpha.valist.Uninitialized \
+    -enable-checker alpha.valist.Unterminated \
+    -enable-checker security.FloatLoopCounter \
     -enable-checker security.insecureAPI.strcpy \
-    -enable-checker alpha.unix.PthreadLock \
-    -enable-checker alpha.core.PointerArithm \
-    -enable-checker alpha.core.TestAfterDivZero \
 "
 
+# These have high false-positive rates.
+EXTRA_CHECKERS="\
+    -enable-checker alpha.security.ArrayBoundV2 \
+    -enable-checker alpha.unix.cstring.OutOfBounds \
+    -enable-checker alpha.core.CastSize \
+"
+
+# These don't seem to generate anything useful
+NOISY_CHECKERS="\
+    -enable-checker alpha.clone.CloneChecker \
+    -enable-checker alpha.deadcode.UnreachableCode \
+"
+
+if test "x$SCAN_BUILD_OUTPUT" != "x"; then
+   OUTPUTARG="-o $SCAN_BUILD_OUTPUT"
+else
+   OUTPUTARG=""
+fi
+
 scan-build \
     $CHECKERS \
     ./configure
 
+make clean
+
 scan-build \
-    $CHECKERS \
-    make -j2 -k
+    $CHECKERS $OUTPUTARG \
+    make -j5 -k
 
+CHECKERS="\
+"
 
 # This one gives a false positive on every strcmp.
 #    -enable-checker alpha.core.PointerSub
 
 # Needs work
-#  alpha.unix.MallocWithAnnotations ??
+#    -enable-checker alpha.unix.MallocWithAnnotations

+ 2 - 0
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)

+ 12 - 13
src/common/compat.c

@@ -2236,34 +2236,32 @@ switch_id(const char *user, const unsigned flags)
 int
 tor_disable_debugger_attach(void)
 {
-  int r, attempted;
-  r = -1;
-  attempted = 0;
+  int r = -1;
   log_debug(LD_CONFIG,
             "Attemping to disable debugger attachment to Tor for "
             "unprivileged users.");
 #if defined(__linux__) && defined(HAVE_SYS_PRCTL_H) \
   && defined(HAVE_PRCTL) && defined(PR_SET_DUMPABLE)
-  attempted = 1;
+#define TRIED_TO_DISABLE
   r = prctl(PR_SET_DUMPABLE, 0);
-#endif
-#if defined(__APPLE__) && defined(PT_DENY_ATTACH)
-  if (r < 0) {
-    attempted = 1;
-    r = ptrace(PT_DENY_ATTACH, 0, 0, 0);
-  }
-#endif /* defined(__APPLE__) && defined(PT_DENY_ATTACH) */
+#elif defined(__APPLE__) && defined(PT_DENY_ATTACH)
+#define TRIED_TO_ATTACH
+  r = ptrace(PT_DENY_ATTACH, 0, 0, 0);
+#endif /* defined(__linux__) && defined(HAVE_SYS_PRCTL_H) ... || ... */
 
   // XXX: TODO - Mac OS X has dtrace and this may be disabled.
   // XXX: TODO - Windows probably has something similar
-  if (r == 0 && attempted) {
+#ifdef TRIED_TO_DISABLE
+  if (r == 0) {
     log_debug(LD_CONFIG,"Debugger attachment disabled for "
               "unprivileged users.");
     return 1;
-  } else if (attempted) {
+  } else {
     log_warn(LD_CONFIG, "Unable to disable debugger attaching: %s",
              strerror(errno));
   }
+#endif /* defined(TRIED_TO_DISABLE) */
+#undef TRIED_TO_DISABLE
   return r;
 }
 
@@ -2582,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)

+ 1 - 1
src/common/timers.c

@@ -191,7 +191,7 @@ timers_initialize(void)
   if (BUG(global_timeouts))
     return; // LCOV_EXCL_LINE
 
-  timeout_error_t err;
+  timeout_error_t err = 0;
   global_timeouts = timeouts_open(0, &err);
   if (!global_timeouts) {
     // LCOV_EXCL_START -- this can only fail on malloc failure.

+ 14 - 13
src/common/util.c

@@ -445,7 +445,7 @@ tor_log2(uint64_t u64)
     r += 2;
   }
   if (u64 >= (U64_LITERAL(1)<<1)) {
-    u64 >>= 1;
+    // u64 >>= 1; // not using this any more.
     r += 1;
   }
   return r;
@@ -3616,7 +3616,7 @@ start_daemon(void)
   } else { /* Child */
     close(daemon_filedes[0]); /* we only write */
 
-    pid = setsid(); /* Detach from controlling terminal */
+    (void) setsid(); /* Detach from controlling terminal */
     /*
      * Fork one more time, so the parent (the session group leader) can exit.
      * This means that we, as a non-session group leader, can never regain a
@@ -4314,7 +4314,6 @@ tor_spawn_background(const char *const filename, const char **argv,
   int stderr_pipe[2];
   int stdin_pipe[2];
   int fd, retval;
-  ssize_t nbytes;
   process_handle_t *process_handle;
   int status;
 
@@ -4335,7 +4334,7 @@ tor_spawn_background(const char *const filename, const char **argv,
      and we are not allowed to use unsafe functions between fork and exec */
   error_message_length = strlen(error_message);
 
-  child_state = CHILD_STATE_PIPE;
+  // child_state = CHILD_STATE_PIPE;
 
   /* Set up pipe for redirecting stdout, stderr, and stdin of child */
   retval = pipe(stdout_pipe);
@@ -4372,7 +4371,7 @@ tor_spawn_background(const char *const filename, const char **argv,
     return status;
   }
 
-  child_state = CHILD_STATE_MAXFD;
+  // child_state = CHILD_STATE_MAXFD;
 
 #ifdef _SC_OPEN_MAX
   if (-1 == max_fd) {
@@ -4387,7 +4386,7 @@ tor_spawn_background(const char *const filename, const char **argv,
   max_fd = DEFAULT_MAX_FD;
 #endif /* defined(_SC_OPEN_MAX) */
 
-  child_state = CHILD_STATE_FORK;
+  // child_state = CHILD_STATE_FORK;
 
   pid = fork();
   if (0 == pid) {
@@ -4423,7 +4422,7 @@ tor_spawn_background(const char *const filename, const char **argv,
     if (-1 == retval)
       goto error;
 
-    child_state = CHILD_STATE_CLOSEFD;
+    // child_state = CHILD_STATE_CLOSEFD;
 
     close(stderr_pipe[0]);
     close(stderr_pipe[1]);
@@ -4439,7 +4438,7 @@ tor_spawn_background(const char *const filename, const char **argv,
       close(fd);
     }
 
-    child_state = CHILD_STATE_EXEC;
+    // child_state = CHILD_STATE_EXEC;
 
     /* Call the requested program. We need the cast because
        execvp doesn't define argv as const, even though it
@@ -4458,7 +4457,8 @@ tor_spawn_background(const char *const filename, const char **argv,
   error:
     {
       /* XXX: are we leaking fds from the pipe? */
-      int n;
+      int n, err=0;
+      ssize_t nbytes;
 
       n = format_helper_exit_status(child_state, errno, hex_errno);
 
@@ -4467,13 +4467,14 @@ tor_spawn_background(const char *const filename, const char **argv,
            value, but there is nothing we can do if it fails */
         /* TODO: Don't use STDOUT, use a pipe set up just for this purpose */
         nbytes = write(STDOUT_FILENO, error_message, error_message_length);
+        err = (nbytes < 0);
         nbytes = write(STDOUT_FILENO, hex_errno, n);
+        err += (nbytes < 0);
       }
-    }
 
-    (void) nbytes;
+      _exit(err?254:255);
+    }
 
-    _exit(255);
     /* Never reached, but avoids compiler warning */
     return status; // LCOV_EXCL_LINE
   }
@@ -4540,7 +4541,7 @@ tor_spawn_background(const char *const filename, const char **argv,
   }
 
   *process_handle_out = process_handle;
-  return process_handle->status;
+  return status;
 #endif /* defined(_WIN32) */
 }
 

+ 3 - 3
src/or/circuitbuild.c

@@ -2422,10 +2422,10 @@ cpath_get_n_hops(crypt_path_t **head_ptr)
   }
 
   tmp = *head_ptr;
-  if (tmp) {
+  do {
     n_hops++;
-    tmp = (*head_ptr)->next;
-  }
+    tmp = tmp->next;
+  } while (tmp != *head_ptr);
 
   return n_hops;
 }

+ 2 - 8
src/or/circuitmux.c

@@ -261,13 +261,11 @@ circuitmux_move_active_circ_to_tail(circuitmux_t *cmux, circuit_t *circ,
     if (circ->n_mux == cmux) {
       next_p = &(circ->next_active_on_n_chan);
       prev_p = &(circ->prev_active_on_n_chan);
-      direction = CELL_DIRECTION_OUT;
     } else {
       or_circ = TO_OR_CIRCUIT(circ);
       tor_assert(or_circ->p_mux == cmux);
       next_p = &(or_circ->next_active_on_p_chan);
       prev_p = &(or_circ->prev_active_on_p_chan);
-      direction = CELL_DIRECTION_IN;
     }
   }
   tor_assert(next_p);
@@ -1655,7 +1653,6 @@ circuitmux_assert_okay_pass_one(circuitmux_t *cmux)
   circid_t circ_id;
   circuit_t *circ;
   or_circuit_t *or_circ;
-  unsigned int circ_is_active;
   circuit_t **next_p, **prev_p;
   unsigned int n_circuits, n_active_circuits, n_cells;
 
@@ -1679,8 +1676,6 @@ circuitmux_assert_okay_pass_one(circuitmux_t *cmux)
     tor_assert(chan);
     circ = circuit_get_by_circid_channel_even_if_marked(circ_id, chan);
     tor_assert(circ);
-    /* Clear the circ_is_active bit to start */
-    circ_is_active = 0;
 
     /* Assert that we know which direction this is going */
     tor_assert((*i)->muxinfo.direction == CELL_DIRECTION_OUT ||
@@ -1707,7 +1702,7 @@ circuitmux_assert_okay_pass_one(circuitmux_t *cmux)
      * Should this circuit be active?  I.e., does the mux know about > 0
      * cells on it?
      */
-    circ_is_active = ((*i)->muxinfo.cell_count > 0);
+    const int circ_is_active = ((*i)->muxinfo.cell_count > 0);
 
     /* It should be in the linked list iff it's active */
     if (circ_is_active) {
@@ -1759,7 +1754,6 @@ circuitmux_assert_okay_pass_two(circuitmux_t *cmux)
   circuit_t **next_p, **prev_p;
   channel_t *chan;
   unsigned int n_active_circuits = 0;
-  cell_direction_t direction;
   chanid_circid_muxinfo_t search, *hashent = NULL;
 
   tor_assert(cmux);
@@ -1778,7 +1772,7 @@ circuitmux_assert_okay_pass_two(circuitmux_t *cmux)
     curr_or_circ = NULL;
     next_circ = NULL;
     next_p = prev_p = NULL;
-    direction = 0;
+    cell_direction_t direction;
 
     /* Figure out if this is n_mux or p_mux */
     if (cmux == curr_circ->n_mux) {

+ 2 - 0
src/or/config.c

@@ -6120,6 +6120,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);

+ 5 - 5
src/or/connection.c

@@ -3062,9 +3062,11 @@ connection_buckets_decrement(connection_t *conn, time_t now,
              (unsigned long)num_read, (unsigned long)num_written,
              conn_type_to_string(conn->type),
              conn_state_to_string(conn->type, conn->state));
-    if (num_written >= INT_MAX) num_written = 1;
-    if (num_read >= INT_MAX) num_read = 1;
-    tor_fragile_assert();
+    tor_assert_nonfatal_unreached();
+    if (num_written >= INT_MAX)
+      num_written = 1;
+    if (num_read >= INT_MAX)
+      num_read = 1;
   }
 
   record_num_bytes_transferred_impl(conn, now, num_read, num_written);
@@ -3593,10 +3595,8 @@ connection_buf_read_from_socket(connection_t *conn, ssize_t *max_to_read,
             connection_start_reading(conn);
         }
         /* we're already reading, one hopes */
-        result = 0;
         break;
       case TOR_TLS_DONE: /* no data read, so nothing to process */
-        result = 0;
         break; /* so we call bucket_decrement below */
       default:
         break;

+ 6 - 6
src/or/directory.c

@@ -3837,7 +3837,6 @@ directory_handle_command_get,(dir_connection_t *conn, const char *headers,
   char *url, *url_mem, *header;
   time_t if_modified_since = 0;
   int zlib_compressed_in_url;
-  size_t url_len;
   unsigned compression_methods_supported;
 
   /* We ignore the body of a GET request. */
@@ -3868,12 +3867,13 @@ directory_handle_command_get,(dir_connection_t *conn, const char *headers,
   log_debug(LD_DIRSERV,"rewritten url as '%s'.", escaped(url));
 
   url_mem = url;
-  url_len = strlen(url);
+  {
+    size_t url_len = strlen(url);
 
-  zlib_compressed_in_url = url_len > 2 && !strcmp(url+url_len-2, ".z");
-  if (zlib_compressed_in_url) {
-    url[url_len-2] = '\0';
-    url_len -= 2;
+    zlib_compressed_in_url = url_len > 2 && !strcmp(url+url_len-2, ".z");
+    if (zlib_compressed_in_url) {
+      url[url_len-2] = '\0';
+    }
   }
 
   if ((header = http_get_header(headers, "Accept-Encoding: "))) {

+ 11 - 10
src/or/dirvote.c

@@ -541,8 +541,8 @@ compute_routerstatus_consensus(smartlist_t *votes, int consensus_method,
   if (cur_n > most_n ||
       (cur && cur_n == most_n && cur->status.published_on > most_published)) {
     most = cur;
-    most_n = cur_n;
-    most_published = cur->status.published_on;
+    // most_n = cur_n; // unused after this point.
+    // most_published = cur->status.published_on; // unused after this point.
   }
 
   tor_assert(most);
@@ -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);
   }

+ 1 - 0
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;

+ 9 - 9
src/or/rendservice.c

@@ -696,7 +696,6 @@ rend_config_service(const config_line_t *line_,
        * of authorized clients. */
       smartlist_t *type_names_split, *clients;
       const char *authname;
-      int num_clients;
       if (service->auth_type != REND_NO_AUTH) {
         log_warn(LD_CONFIG, "Got multiple HiddenServiceAuthorizeClient "
                  "lines for a single service.");
@@ -740,14 +739,15 @@ rend_config_service(const config_line_t *line_,
       SMARTLIST_FOREACH(type_names_split, char *, cp, tor_free(cp));
       smartlist_free(type_names_split);
       /* Remove duplicate client names. */
-      num_clients = smartlist_len(clients);
-      smartlist_sort_strings(clients);
-      smartlist_uniq_strings(clients);
-      if (smartlist_len(clients) < num_clients) {
-        log_info(LD_CONFIG, "HiddenServiceAuthorizeClient contains %d "
-                            "duplicate client name(s); removing.",
-                 num_clients - smartlist_len(clients));
-        num_clients = smartlist_len(clients);
+      {
+        int num_clients = smartlist_len(clients);
+        smartlist_sort_strings(clients);
+        smartlist_uniq_strings(clients);
+        if (smartlist_len(clients) < num_clients) {
+          log_info(LD_CONFIG, "HiddenServiceAuthorizeClient contains %d "
+                   "duplicate client name(s); removing.",
+                   num_clients - smartlist_len(clients));
+        }
       }
       SMARTLIST_FOREACH_BEGIN(clients, const char *, client_name)
       {

+ 3 - 3
src/or/routerparse.c

@@ -2934,7 +2934,7 @@ networkstatus_verify_bw_weights(networkstatus_t *ns, int consensus_method)
   }
 
   Wgg /= weight_scale;
-  Wgm /= weight_scale;
+  Wgm /= weight_scale; (void) Wgm; // unused from here on.
   Wgd /= weight_scale;
 
   Wmg /= weight_scale;
@@ -2942,8 +2942,8 @@ networkstatus_verify_bw_weights(networkstatus_t *ns, int consensus_method)
   Wme /= weight_scale;
   Wmd /= weight_scale;
 
-  Weg /= weight_scale;
-  Wem /= weight_scale;
+  Weg /= weight_scale; (void) Weg; // unused from here on.
+  Wem /= weight_scale; (void) Wem; // unused from here on.
   Wee /= weight_scale;
   Wed /= weight_scale;
 

+ 0 - 2
src/or/statefile.c

@@ -659,8 +659,6 @@ save_transport_to_state(const char *transport,
     *next = line = tor_malloc_zero(sizeof(config_line_t));
     line->key = tor_strdup("TransportProxy");
     tor_asprintf(&line->value, "%s %s", transport, fmt_addrport(addr, port));
-
-    next = &(line->next);
   }
 
   if (!get_options()->AvoidDiskWrites)

+ 1 - 0
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();

+ 2 - 0
src/test/test_addr.c

@@ -760,7 +760,9 @@ test_addr_ip6_helpers(void *arg)
 
   /* get interface addresses */
   r = get_interface_address6(LOG_DEBUG, AF_INET, &t1);
+  tt_int_op(r, OP_LE, 0); // "it worked or it didn't"
   i = get_interface_address6(LOG_DEBUG, AF_INET6, &t2);
+  tt_int_op(i, OP_LE, 0); // "it worked or it didn't"
 
   TT_BLATHER(("v4 address: %s (family=%d)", fmt_addr(&t1),
               tor_addr_family(&t1)));

+ 1 - 0
src/test/test_config.c

@@ -704,6 +704,7 @@ test_config_parse_transport_plugin_line(void *arg)
   tt_int_op(r, OP_LT, 0);
   r = parse_transport_line(options,
       "transport_1,transport_2 proxy 1.2.3.4:567", 1, 1);
+  tt_int_op(r, OP_LT, 0);
   /* No port error exit */
   r = parse_transport_line(options,
       "transport_1 socks5 1.2.3.4", 1, 0);

+ 10 - 9
src/test/test_consdiff.c

@@ -19,28 +19,29 @@ test_consdiff_smartlist_slice(void *arg)
 {
   smartlist_t *sl = smartlist_new();
   smartlist_slice_t *sls;
+  int items[6] = {0,0,0,0,0,0};
 
   /* Create a regular smartlist. */
   (void)arg;
-  smartlist_add(sl, (void*)1);
-  smartlist_add(sl, (void*)2);
-  smartlist_add(sl, (void*)3);
-  smartlist_add(sl, (void*)4);
-  smartlist_add(sl, (void*)5);
+  smartlist_add(sl, &items[1]);
+  smartlist_add(sl, &items[2]);
+  smartlist_add(sl, &items[3]);
+  smartlist_add(sl, &items[4]);
+  smartlist_add(sl, &items[5]);
 
   /* See if the slice was done correctly. */
   sls = smartlist_slice(sl, 2, 5);
   tt_ptr_op(sl, OP_EQ, sls->list);
-  tt_ptr_op((void*)3, OP_EQ, smartlist_get(sls->list, sls->offset));
-  tt_ptr_op((void*)5, OP_EQ,
+  tt_ptr_op(&items[3], OP_EQ, smartlist_get(sls->list, sls->offset));
+  tt_ptr_op(&items[5], OP_EQ,
       smartlist_get(sls->list, sls->offset + (sls->len-1)));
   tor_free(sls);
 
   /* See that using -1 as the end does get to the last element. */
   sls = smartlist_slice(sl, 2, -1);
   tt_ptr_op(sl, OP_EQ, sls->list);
-  tt_ptr_op((void*)3, OP_EQ, smartlist_get(sls->list, sls->offset));
-  tt_ptr_op((void*)5, OP_EQ,
+  tt_ptr_op(&items[3], OP_EQ, smartlist_get(sls->list, sls->offset));
+  tt_ptr_op(&items[5], OP_EQ,
       smartlist_get(sls->list, sls->offset + (sls->len-1)));
 
  done:

+ 3 - 0
src/test/test_crypto.c

@@ -1406,6 +1406,7 @@ test_crypto_digests(void *arg)
              AUTHORITY_SIGNKEY_A_DIGEST, HEX_DIGEST_LEN);
 
   r = crypto_pk_get_common_digests(k, &pkey_digests);
+  tt_int_op(r, OP_EQ, 0);
 
   tt_mem_op(hex_str(pkey_digests.d[DIGEST_SHA1], DIGEST_LEN),OP_EQ,
              AUTHORITY_SIGNKEY_A_DIGEST, HEX_DIGEST_LEN);
@@ -2594,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))
 

+ 5 - 1
src/test/test_dir.c

@@ -2252,6 +2252,7 @@ test_dir_networkstatus_compute_bw_weights_v10(void *arg)
   tt_i64_op(G+M+E+D, OP_EQ, T);
   ret = networkstatus_compute_bw_weights_v10(chunks, G, M, E, D, T,
                                              weight_scale);
+  tt_assert(ret);
   tt_str_op(smartlist_get(chunks, 0), OP_EQ, "bandwidth-weights Wbd=883 Wbe=0 "
     "Wbg=3673 Wbm=10000 Wdb=10000 Web=10000 Wed=8233 Wee=10000 Weg=8233 "
     "Wem=10000 Wgb=10000 Wgd=883 Wgg=6327 Wgm=6327 Wmb=10000 Wmd=883 Wme=0 "
@@ -2268,6 +2269,7 @@ test_dir_networkstatus_compute_bw_weights_v10(void *arg)
   tt_i64_op(G+M+E+D, OP_EQ, T);
   ret = networkstatus_compute_bw_weights_v10(chunks, G, M, E, D, T,
                                              weight_scale);
+  tt_assert(ret);
   tt_str_op(smartlist_get(chunks, 0), OP_EQ, "bandwidth-weights Wbd=0 Wbe=0 "
     "Wbg=4194 Wbm=10000 Wdb=10000 Web=10000 Wed=10000 Wee=10000 Weg=10000 "
     "Wem=10000 Wgb=10000 Wgd=0 Wgg=5806 Wgm=5806 Wmb=10000 Wmd=0 Wme=0 "
@@ -2284,6 +2286,7 @@ test_dir_networkstatus_compute_bw_weights_v10(void *arg)
   tt_i64_op(G+M+E+D, OP_EQ, T);
   ret = networkstatus_compute_bw_weights_v10(chunks, G, M, E, D, T,
                                              weight_scale);
+  tt_assert(ret);
   tt_str_op(smartlist_get(chunks, 0), OP_EQ, "bandwidth-weights Wbd=317 "
     "Wbe=5938 Wbg=0 Wbm=10000 Wdb=10000 Web=10000 Wed=9366 Wee=4061 "
     "Weg=9366 Wem=4061 Wgb=10000 Wgd=317 Wgg=10000 Wgm=10000 Wmb=10000 "
@@ -2304,6 +2307,7 @@ test_dir_networkstatus_compute_bw_weights_v10(void *arg)
     "Wbe=0 Wbg=0 Wbm=10000 Wdb=10000 Web=10000 Wed=3333 Wee=10000 Weg=3333 "
     "Wem=10000 Wgb=10000 Wgd=3333 Wgg=10000 Wgm=10000 Wmb=10000 Wmd=3333 "
     "Wme=0 Wmg=0 Wmm=10000\n");
+  tt_assert(ret);
 
  done:
   SMARTLIST_FOREACH(chunks, char *, cp, tor_free(cp));
@@ -3366,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),

+ 5 - 3
src/test/test_hs_client.c

@@ -218,7 +218,8 @@ test_e2e_rend_circuit_setup_legacy(void *arg)
 
  done:
   connection_free_(conn);
-  tor_free(TO_CIRCUIT(or_circ)->n_chan);
+  if (or_circ)
+    tor_free(TO_CIRCUIT(or_circ)->n_chan);
   circuit_free(TO_CIRCUIT(or_circ));
 }
 
@@ -227,7 +228,7 @@ static void
 test_e2e_rend_circuit_setup(void *arg)
 {
   uint8_t ntor_key_seed[DIGEST256_LEN] = {0};
-  origin_circuit_t *or_circ;
+  origin_circuit_t *or_circ = NULL;
   int retval;
   connection_t *conn = NULL;
 
@@ -287,7 +288,8 @@ test_e2e_rend_circuit_setup(void *arg)
 
  done:
   connection_free_(conn);
-  tor_free(TO_CIRCUIT(or_circ)->n_chan);
+  if (or_circ)
+    tor_free(TO_CIRCUIT(or_circ)->n_chan);
   circuit_free(TO_CIRCUIT(or_circ));
 }
 

+ 2 - 1
src/test/test_hs_common.c

@@ -716,6 +716,7 @@ test_hid_serv_request_tracker(void *arg)
 
   /* Add another request with very short key */
   retval = hs_lookup_last_hid_serv_request(hsdir, "l",  now, 1);
+  tt_int_op(retval, OP_EQ, now);
   tt_int_op(strmap_size(request_tracker),OP_EQ, 3);
 
   /* Try deleting entries with a dummy key. Check that our previous requests
@@ -1588,7 +1589,7 @@ helper_test_hsdir_sync(networkstatus_t *ns,
   tt_int_op(smartlist_len(desc->previous_hsdirs), OP_EQ, 6);
 
   /* 3) Initialize client time */
-  now = helper_set_consensus_and_system_time(ns, client_position);
+  helper_set_consensus_and_system_time(ns, client_position);
 
   cleanup_nodelist();
   SMARTLIST_FOREACH(ns->routerstatus_list,

+ 1 - 0
src/test/test_hs_descriptor.c

@@ -90,6 +90,7 @@ test_cert_encoding(void *arg)
     pos += b64_cert_len;
     tt_int_op(strcmpstart(pos, "-----END ED25519 CERT-----"), OP_EQ, 0);
     pos += strlen("-----END ED25519 CERT-----");
+    tt_str_op(pos, OP_EQ, "");
   }
 
  done:

+ 3 - 1
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));

+ 11 - 5
src/test/test_hs_service.c

@@ -425,7 +425,7 @@ test_service_intro_point(void *arg)
   /* Test functions that uses a service intropoints map with that previously
    * created object (non legacy). */
   {
-    uint8_t garbage[DIGEST256_LEN] = {0};
+    ed25519_public_key_t garbage = { {0} };
     hs_service_intro_point_t *query;
 
     service = hs_service_new(get_options());
@@ -436,8 +436,7 @@ test_service_intro_point(void *arg)
     service_intro_point_add(service->desc_current->intro_points.map, ip);
     query = service_intro_point_find(service, &ip->auth_key_kp.pubkey);
     tt_mem_op(query, OP_EQ, ip, sizeof(hs_service_intro_point_t));
-    query = service_intro_point_find(service,
-                                     (const ed25519_public_key_t *) garbage);
+    query = service_intro_point_find(service, &garbage);
     tt_ptr_op(query, OP_EQ, NULL);
 
     /* While at it, can I find the descriptor with the intro point? */
@@ -677,6 +676,8 @@ test_intro_established(void *arg)
 
   circ = helper_create_origin_circuit(CIRCUIT_PURPOSE_S_ESTABLISH_INTRO,
                                       flags);
+  tt_assert(circ);
+
   /* Test a wrong purpose. */
   TO_CIRCUIT(circ)->purpose = CIRCUIT_PURPOSE_S_INTRO;
   setup_full_capture_of_logs(LOG_WARN);
@@ -724,7 +725,8 @@ test_intro_established(void *arg)
   tt_int_op(TO_CIRCUIT(circ)->purpose, OP_EQ, CIRCUIT_PURPOSE_S_INTRO);
 
  done:
-  circuit_free(TO_CIRCUIT(circ));
+  if (circ)
+    circuit_free(TO_CIRCUIT(circ));
   hs_free_all();
   UNMOCK(circuit_mark_for_close_);
 }
@@ -793,6 +795,7 @@ test_introduce2(void *arg)
   dummy_state = tor_malloc_zero(sizeof(or_state_t));
 
   circ = helper_create_origin_circuit(CIRCUIT_PURPOSE_S_INTRO, flags);
+  tt_assert(circ);
 
   /* Test a wrong purpose. */
   TO_CIRCUIT(circ)->purpose = CIRCUIT_PURPOSE_S_ESTABLISH_INTRO;
@@ -844,7 +847,8 @@ test_introduce2(void *arg)
  done:
   or_state_free(dummy_state);
   dummy_state = NULL;
-  circuit_free(TO_CIRCUIT(circ));
+  if (circ)
+    circuit_free(TO_CIRCUIT(circ));
   hs_free_all();
   UNMOCK(circuit_mark_for_close_);
 }
@@ -1264,6 +1268,7 @@ test_upload_descriptors(void *arg)
 
   ret = parse_rfc1123_time("Sat, 26 Oct 1985 13:00:00 UTC",
                            &mock_ns.valid_after);
+  tt_int_op(ret, OP_EQ, 0);
   ret = parse_rfc1123_time("Sat, 26 Oct 1985 14:00:00 UTC",
                            &mock_ns.fresh_until);
   tt_int_op(ret, OP_EQ, 0);
@@ -1339,6 +1344,7 @@ test_revision_counter_state(void *arg)
                                                  &desc_two->blinded_kp.pubkey,
                                                                &service_found);
   tt_int_op(service_found, OP_EQ, 0);
+  tt_u64_op(cached_rev_counter, OP_EQ, 0);
 
   /* Now let's try with the right pubkeys */
   cached_rev_counter =check_state_line_for_service_rev_counter(state_line_one,

+ 6 - 0
src/test/test_options.c

@@ -659,6 +659,7 @@ test_options_validate__logs(void *ignored)
   tt_str_op(tdata->opt->Logs->key, OP_EQ, "Log");
   tt_str_op(tdata->opt->Logs->value, OP_EQ, "notice stdout");
   tor_free(msg);
+  tt_int_op(ret, OP_EQ, -1);
 
   free_options_test_data(tdata);
   tdata = get_options_test_data("");
@@ -669,6 +670,7 @@ test_options_validate__logs(void *ignored)
   tt_str_op(tdata->opt->Logs->key, OP_EQ, "Log");
   tt_str_op(tdata->opt->Logs->value, OP_EQ, "warn stdout");
   tor_free(msg);
+  tt_int_op(ret, OP_EQ, -1);
 
   free_options_test_data(tdata);
   tdata = get_options_test_data("");
@@ -678,6 +680,7 @@ test_options_validate__logs(void *ignored)
   ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg);
   tt_assert(!tdata->opt->Logs);
   tor_free(msg);
+  tt_int_op(ret, OP_EQ, -1);
 
   free_options_test_data(tdata);
   tdata = get_options_test_data("");
@@ -686,6 +689,7 @@ test_options_validate__logs(void *ignored)
   ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 1, &msg);
   tt_assert(!tdata->opt->Logs);
   tor_free(msg);
+  tt_int_op(ret, OP_EQ, -1);
 
   free_options_test_data(tdata);
   tdata = get_options_test_data("");
@@ -694,6 +698,7 @@ test_options_validate__logs(void *ignored)
   ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg);
   tt_assert(!tdata->opt->Logs);
   tor_free(msg);
+  tt_int_op(ret, OP_EQ, -1);
 
   free_options_test_data(tdata);
   tdata = get_options_test_data("");
@@ -703,6 +708,7 @@ test_options_validate__logs(void *ignored)
   tdata->opt->Logs = cl;
   ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg);
   tt_int_op((intptr_t)tdata->opt->Logs, OP_EQ, (intptr_t)cl);
+  tt_int_op(ret, OP_EQ, -1);
 
  done:
   quiet_level = orig_quiet_level;

+ 1 - 0
src/test/test_rendcache.c

@@ -58,6 +58,7 @@ test_rend_cache_lookup_entry(void *data)
   tt_int_op(ret, OP_EQ, 0);
 
   ret = rend_cache_lookup_entry(service_id, 2, &entry);
+  tt_int_op(ret, OP_EQ, 0);
   tt_assert(entry);
   tt_int_op(entry->len, OP_EQ, strlen(desc_holder->desc_str));
   tt_str_op(entry->desc, OP_EQ, desc_holder->desc_str);

+ 2 - 0
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. */

+ 1 - 0
src/test/test_tortls.c

@@ -1378,6 +1378,7 @@ test_tortls_evaluate_ecgroup_for_tls(void *ignored)
 
   ret = evaluate_ecgroup_for_tls("P224");
   //  tt_int_op(ret, OP_EQ, 1); This varies between machines
+  tt_assert(ret == 0 || ret == 1);
 
  done:
   (void)0;

+ 3 - 0
src/test/test_util.c

@@ -2179,10 +2179,13 @@ test_util_parse_integer(void *arg)
   tt_int_op(1,OP_EQ, i);
   tt_assert(DBL_TO_U64(d) == 0);
   d = tor_parse_double(" ", 0, (double)UINT64_MAX,&i,NULL);
+  tt_double_op(fabs(d), OP_LT, 1e-10);
   tt_int_op(0,OP_EQ, i);
   d = tor_parse_double(".0a", 0, (double)UINT64_MAX,&i,NULL);
+  tt_double_op(fabs(d), OP_LT, 1e-10);
   tt_int_op(0,OP_EQ, i);
   d = tor_parse_double(".0a", 0, (double)UINT64_MAX,&i,&cp);
+  tt_double_op(fabs(d), OP_LT, 1e-10);
   tt_int_op(1,OP_EQ, i);
   d = tor_parse_double("-.0", 0, (double)UINT64_MAX,&i,NULL);
   tt_int_op(1,OP_EQ, i);

+ 8 - 4
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;