Merge branch 'coverity_20140821'

14 files changed, 83 insertions, 56 deletions

diff --git a/src/common/container.c b/src/common/container.c
index 54f041bac7..7f02dec550 100644
--- a/src/common/container.c
+++ b/src/common/container.c
@@ -66,11 +66,17 @@ smartlist_ensure_capacity(smartlist_t *sl, int size)
 #define MAX_CAPACITY (INT_MAX)
 #else
 #define MAX_CAPACITY (int)((SIZE_MAX / (sizeof(void*))))
+#define ASSERT_CAPACITY
 #endif
   if (size > sl->capacity) {
     int higher = sl->capacity;
     if (PREDICT_UNLIKELY(size > MAX_CAPACITY/2)) {
+#ifdef ASSERT_CAPACITY
+      /* We don't include this assertion when MAX_CAPACITY == INT_MAX,
+       * since int size; (size <= INT_MAX) makes analysis tools think we're
+       * doing something stupid. */
       tor_assert(size <= MAX_CAPACITY);
+#endif
       higher = MAX_CAPACITY;
     } else {
       while (size > higher)
@@ -80,6 +86,8 @@ smartlist_ensure_capacity(smartlist_t *sl, int size)
     sl->list = tor_reallocarray(sl->list, sizeof(void *),
                                 ((size_t)sl->capacity));
   }
+#undef ASSERT_CAPACITY
+#undef MAX_CAPACITY
 }
 
 /** Append element to the end of the list. */
diff --git a/src/common/di_ops.c b/src/common/di_ops.c
index 14a1443400..a8bfd02532 100644
--- a/src/common/di_ops.c
+++ b/src/common/di_ops.c
@@ -130,6 +130,7 @@ tor_memeq(const void *a, const void *b, size_t sz)
    *            1 & ((any_difference - 1) >> 8) == 0
    */
 
+  /*coverity[overflow]*/
   return 1 & ((any_difference - 1) >> 8);
 }
 
@@ -217,6 +218,7 @@ safe_mem_is_zero(const void *mem, size_t sz)
     total |= *ptr++;
   }
 
+  /*coverity[overflow]*/
   return 1 & ((total - 1) >> 8);
 }
 
diff --git a/src/common/sandbox.c b/src/common/sandbox.c
index 05b91be7be..e735feb018 100644
--- a/src/common/sandbox.c
+++ b/src/common/sandbox.c
@@ -98,6 +98,8 @@ static sandbox_cfg_t *filter_dynamic = NULL;
 
 #undef SCMP_CMP
 #define SCMP_CMP(a,b,c) ((struct scmp_arg_cmp){(a),(b),(c),0})
+#define SCMP_CMP_STR(a,b,c) \
+  ((struct scmp_arg_cmp){(a),(b),(intptr_t)(void*)(c),0})
 #define SCMP_CMP4(a,b,c,d) ((struct scmp_arg_cmp){(a),(b),(c),(d)})
 /* We use a wrapper here because these masked comparisons seem to be pretty
  * verbose. Also, it's important to cast to scmp_datum_t before negating the
@@ -252,7 +254,7 @@ sb_execve(scmp_filter_ctx ctx, sandbox_cfg_t *filter)
     if (param != NULL && param->prot == 1 && param->syscall
         == SCMP_SYS(execve)) {
       rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(execve),
-          SCMP_CMP(0, SCMP_CMP_EQ, param->value));
+               SCMP_CMP_STR(0, SCMP_CMP_EQ, param->value));
       if (rc != 0) {
         log_err(LD_BUG,"(Sandbox) failed to add execve syscall, received "
             "libseccomp error %d", rc);
@@ -389,7 +391,7 @@ sb_open(scmp_filter_ctx ctx, sandbox_cfg_t *filter)
     if (param != NULL && param->prot == 1 && param->syscall
         == SCMP_SYS(open)) {
       rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(open),
-            SCMP_CMP(0, SCMP_CMP_EQ, param->value));
+            SCMP_CMP_STR(0, SCMP_CMP_EQ, param->value));
       if (rc != 0) {
         log_err(LD_BUG,"(Sandbox) failed to add open syscall, received "
             "libseccomp error %d", rc);
@@ -444,8 +446,8 @@ sb_rename(scmp_filter_ctx ctx, sandbox_cfg_t *filter)
         param->syscall == SCMP_SYS(rename)) {
 
       rc = seccomp_rule_add_2(ctx, SCMP_ACT_ALLOW, SCMP_SYS(rename),
-            SCMP_CMP(0, SCMP_CMP_EQ, param->value),
-            SCMP_CMP(1, SCMP_CMP_EQ, param->value2));
+            SCMP_CMP_STR(0, SCMP_CMP_EQ, param->value),
+            SCMP_CMP_STR(1, SCMP_CMP_EQ, param->value2));
       if (rc != 0) {
         log_err(LD_BUG,"(Sandbox) failed to add rename syscall, received "
             "libseccomp error %d", rc);
@@ -475,7 +477,7 @@ sb_openat(scmp_filter_ctx ctx, sandbox_cfg_t *filter)
         == SCMP_SYS(openat)) {
       rc = seccomp_rule_add_3(ctx, SCMP_ACT_ALLOW, SCMP_SYS(openat),
           SCMP_CMP(0, SCMP_CMP_EQ, AT_FDCWD),
-          SCMP_CMP(1, SCMP_CMP_EQ, param->value),
+          SCMP_CMP_STR(1, SCMP_CMP_EQ, param->value),
           SCMP_CMP(2, SCMP_CMP_EQ, O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|
               O_CLOEXEC));
       if (rc != 0) {
@@ -884,7 +886,7 @@ sb_stat64(scmp_filter_ctx ctx, sandbox_cfg_t *filter)
     if (param != NULL && param->prot == 1 && (param->syscall == SCMP_SYS(open)
         || param->syscall == SCMP_SYS(stat64))) {
       rc = seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(stat64),
-          SCMP_CMP(0, SCMP_CMP_EQ, param->value));
+          SCMP_CMP_STR(0, SCMP_CMP_EQ, param->value));
       if (rc != 0) {
         log_err(LD_BUG,"(Sandbox) failed to add open syscall, received "
             "libseccomp  error %d", rc);
@@ -967,7 +969,7 @@ static int
 prot_strings_helper(strmap_t *locations,
                     char **pr_mem_next_p,
                     size_t *pr_mem_left_p,
-                    intptr_t *value_p)
+                    char **value_p)
 {
   char *param_val;
   size_t param_size;
@@ -983,7 +985,7 @@ prot_strings_helper(strmap_t *locations,
   if (location) {
     // We already interned this string.
     tor_free(param_val);
-    *value_p = (intptr_t) location;
+    *value_p = location;
     return 0;
   } else if (*pr_mem_left_p >= param_size) {
     // copy to protected
@@ -992,7 +994,7 @@ prot_strings_helper(strmap_t *locations,
 
     // re-point el parameter to protected
     tor_free(param_val);
-    *value_p = (intptr_t) location;
+    *value_p = location;
 
     strmap_set(locations, location, location); /* good real estate advice */
 
@@ -1082,7 +1084,7 @@ prot_strings(scmp_filter_ctx ctx, sandbox_cfg_t* cfg)
         SCMP_CMP(0, SCMP_CMP_EQ, (intptr_t) pr_mem_base));
   if (ret) {
     log_err(LD_BUG,"(Sandbox) munmap protected memory filter fail!");
-    return ret;
+    goto out;
   }
 
   /*
@@ -1101,7 +1103,7 @@ prot_strings(scmp_filter_ctx ctx, sandbox_cfg_t* cfg)
       SCMP_CMP(2, SCMP_CMP_EQ, PROT_READ|PROT_WRITE));
   if (ret) {
     log_err(LD_BUG,"(Sandbox) mprotect protected memory filter fail (LT)!");
-    return ret;
+    goto out;
   }
 
   ret = seccomp_rule_add_3(ctx, SCMP_ACT_ALLOW, SCMP_SYS(mprotect),
@@ -1111,7 +1113,7 @@ prot_strings(scmp_filter_ctx ctx, sandbox_cfg_t* cfg)
       SCMP_CMP(2, SCMP_CMP_EQ, PROT_READ|PROT_WRITE));
   if (ret) {
     log_err(LD_BUG,"(Sandbox) mprotect protected memory filter fail (GT)!");
-    return ret;
+    goto out;
   }
 
  out:
@@ -1126,7 +1128,7 @@ prot_strings(scmp_filter_ctx ctx, sandbox_cfg_t* cfg)
  * point.
  */
 static sandbox_cfg_t*
-new_element2(int syscall, intptr_t value, intptr_t value2)
+new_element2(int syscall, char *value, char *value2)
 {
   smp_param_t *param = NULL;
 
@@ -1142,9 +1144,9 @@ new_element2(int syscall, intptr_t value, intptr_t value2)
 }
 
 static sandbox_cfg_t*
-new_element(int syscall, intptr_t value)
+new_element(int syscall, char *value)
 {
-  return new_element2(syscall, value, 0);
+  return new_element2(syscall, value, NULL);
 }
 
 #ifdef __NR_stat64
@@ -1158,7 +1160,7 @@ sandbox_cfg_allow_stat_filename(sandbox_cfg_t **cfg, char *file)
 {
   sandbox_cfg_t *elem = NULL;
 
-  elem = new_element(SCMP_stat, (intptr_t)(void*) file);
+  elem = new_element(SCMP_stat, file);
   if (!elem) {
     log_err(LD_BUG,"(Sandbox) failed to register parameter!");
     return -1;
@@ -1197,7 +1199,7 @@ sandbox_cfg_allow_open_filename(sandbox_cfg_t **cfg, char *file)
 {
   sandbox_cfg_t *elem = NULL;
 
-  elem = new_element(SCMP_SYS(open), (intptr_t)(void *) file);
+  elem = new_element(SCMP_SYS(open), file);
   if (!elem) {
     log_err(LD_BUG,"(Sandbox) failed to register parameter!");
     return -1;
@@ -1214,9 +1216,7 @@ sandbox_cfg_allow_rename(sandbox_cfg_t **cfg, char *file1, char *file2)
 {
   sandbox_cfg_t *elem = NULL;
 
-  elem = new_element2(SCMP_SYS(rename),
-                      (intptr_t)(void *) file1,
-                      (intptr_t)(void *) file2);
+  elem = new_element2(SCMP_SYS(rename), file1, file2);
 
   if (!elem) {
     log_err(LD_BUG,"(Sandbox) failed to register parameter!");
@@ -1256,7 +1256,7 @@ sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file)
 {
   sandbox_cfg_t *elem = NULL;
 
-  elem = new_element(SCMP_SYS(openat), (intptr_t)(void *) file);
+  elem = new_element(SCMP_SYS(openat), file);
   if (!elem) {
     log_err(LD_BUG,"(Sandbox) failed to register parameter!");
     return -1;
@@ -1296,7 +1296,7 @@ sandbox_cfg_allow_execve(sandbox_cfg_t **cfg, const char *com)
 {
   sandbox_cfg_t *elem = NULL;
 
-  elem = new_element(SCMP_SYS(execve), (intptr_t)(void *) com);
+  elem = new_element(SCMP_SYS(execve), com);
   if (!elem) {
     log_err(LD_BUG,"(Sandbox) failed to register parameter!");
     return -1;
diff --git a/src/common/sandbox.h b/src/common/sandbox.h
index 20d5d5080c..b4bb294750 100644
--- a/src/common/sandbox.h
+++ b/src/common/sandbox.h
@@ -66,9 +66,9 @@ typedef struct smp_param {
   int syscall;
 
   /** parameter value. */
-  intptr_t value;
+  char *value;
   /** parameter value, second argument. */
-  intptr_t value2;
+  char *value2;
 
   /**  parameter flag (0 = not protected, 1 = protected). */
   int prot;
diff --git a/src/common/tortls.c b/src/common/tortls.c
index 0f989684cf..5fe8d81c59 100644
--- a/src/common/tortls.c
+++ b/src/common/tortls.c
@@ -2611,16 +2611,20 @@ check_no_tls_errors_(const char *fname, int line)
 int
 tor_tls_used_v1_handshake(tor_tls_t *tls)
 {
+#if defined(V2_HANDSHAKE_SERVER) && defined(V2_HANDSHAKE_CLIENT)
+  return ! tls->wasV2Handshake;
+#else
   if (tls->isServer) {
-#ifdef V2_HANDSHAKE_SERVER
+# ifdef V2_HANDSHAKE_SERVER
     return ! tls->wasV2Handshake;
-#endif
+# endif
   } else {
-#ifdef V2_HANDSHAKE_CLIENT
+# ifdef V2_HANDSHAKE_CLIENT
     return ! tls->wasV2Handshake;
-#endif
+# endif
   }
   return 1;
+#endif
 }
 
 /** Return true iff <b>name</b> is a DN of a kind that could only
diff --git a/src/common/util.c b/src/common/util.c
index 4029512978..16ff8e3a80 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -4052,8 +4052,11 @@ tor_spawn_background(const char *const filename, const char **argv,
 
   status = process_handle->status = PROCESS_STATUS_RUNNING;
   /* Set stdout/stderr pipes to be non-blocking */
-  fcntl(process_handle->stdout_pipe, F_SETFL, O_NONBLOCK);
-  fcntl(process_handle->stderr_pipe, F_SETFL, O_NONBLOCK);
+  if (fcntl(process_handle->stdout_pipe, F_SETFL, O_NONBLOCK) < 0 ||
+      fcntl(process_handle->stderr_pipe, F_SETFL, O_NONBLOCK) < 0) {
+    log_warn(LD_GENERAL, "Failed to set stderror/stdout pipes nonblocking "
+             "in parent process: %s", strerror(errno));
+  }
   /* Open the buffered IO streams */
   process_handle->stdout_handle = fdopen(process_handle->stdout_pipe, "r");
   process_handle->stderr_handle = fdopen(process_handle->stderr_pipe, "r");
diff --git a/src/or/circuitstats.c b/src/or/circuitstats.c
index 5cdd534507..88a1f9b46c 100644
--- a/src/or/circuitstats.c
+++ b/src/or/circuitstats.c
@@ -1371,10 +1371,11 @@ circuit_build_times_network_check_changed(circuit_build_times_t *cbt)
   }
   cbt->liveness.after_firsthop_idx = 0;
 
+#define MAX_TIMEOUT ((int32_t) (INT32_MAX/2))
   /* Check to see if this has happened before. If so, double the timeout
    * to give people on abysmally bad network connections a shot at access */
   if (cbt->timeout_ms >= circuit_build_times_get_initial_timeout()) {
-    if (cbt->timeout_ms > INT32_MAX/2 || cbt->close_ms > INT32_MAX/2) {
+    if (cbt->timeout_ms > MAX_TIMEOUT || cbt->close_ms > MAX_TIMEOUT) {
       log_warn(LD_CIRC, "Insanely large circuit build timeout value. "
               "(timeout = %fmsec, close = %fmsec)",
                cbt->timeout_ms, cbt->close_ms);
@@ -1386,6 +1387,7 @@ circuit_build_times_network_check_changed(circuit_build_times_t *cbt)
     cbt->close_ms = cbt->timeout_ms
                   = circuit_build_times_get_initial_timeout();
   }
+#undef MAX_TIMEOUT
 
   cbt_control_event_buildtimeout_set(cbt, BUILDTIMEOUT_SET_EVENT_RESET);
 
diff --git a/src/or/control.c b/src/or/control.c
index a920ac835e..b3a9dd693e 100644
--- a/src/or/control.c
+++ b/src/or/control.c
@@ -1039,7 +1039,7 @@ handle_control_authenticate(control_connection_t *conn, uint32_t len,
 {
   int used_quoted_string = 0;
   const or_options_t *options = get_options();
-  const char *errstr = NULL;
+  const char *errstr = "Unknown error";
   char *password;
   size_t password_len;
   const char *cp;
@@ -1160,9 +1160,10 @@ handle_control_authenticate(control_connection_t *conn, uint32_t len,
     }
     if (bad) {
       if (!also_cookie) {
-        log_warn(LD_CONTROL,
+        log_warn(LD_BUG,
                  "Couldn't decode HashedControlPassword: invalid base16");
         errstr="Couldn't decode HashedControlPassword value in configuration.";
+        goto err;
       }
       bad_password = 1;
       SMARTLIST_FOREACH(sl, char *, cp, tor_free(cp));
@@ -1198,8 +1199,7 @@ handle_control_authenticate(control_connection_t *conn, uint32_t len,
 
  err:
   tor_free(password);
-  connection_printf_to_buf(conn, "515 Authentication failed: %s\r\n",
-                           errstr ? errstr : "Unknown reason.");
+  connection_printf_to_buf(conn, "515 Authentication failed: %s\r\n", errstr);
   connection_mark_for_close(TO_CONN(conn));
   return 0;
  ok:
diff --git a/src/or/dirserv.c b/src/or/dirserv.c
index f33437ff52..52258e875f 100644
--- a/src/or/dirserv.c
+++ b/src/or/dirserv.c
@@ -1959,13 +1959,12 @@ routerstatus_format_entry(const routerstatus_t *rs, const char *version,
   char published[ISO_TIME_LEN+1];
   char identity64[BASE64_DIGEST_LEN+1];
   char digest64[BASE64_DIGEST_LEN+1];
-  smartlist_t *chunks = NULL;
+  smartlist_t *chunks = smartlist_new();
 
   format_iso_time(published, rs->published_on);
   digest_to_base64(identity64, rs->identity_digest);
   digest_to_base64(digest64, rs->descriptor_digest);
 
-  chunks = smartlist_new();
   smartlist_add_asprintf(chunks,
                    "r %s %s %s%s%s %s %d %d\n",
                    rs->nickname,
@@ -2090,10 +2089,8 @@ routerstatus_format_entry(const routerstatus_t *rs, const char *version,
   result = smartlist_join_strings(chunks, "", 0, NULL);
 
  err:
-  if (chunks) {
-    SMARTLIST_FOREACH(chunks, char *, cp, tor_free(cp));
-    smartlist_free(chunks);
-  }
+  SMARTLIST_FOREACH(chunks, char *, cp, tor_free(cp));
+  smartlist_free(chunks);
 
   return result;
 }
diff --git a/src/or/dirvote.c b/src/or/dirvote.c
index 30f132115b..cf6b88eb31 100644
--- a/src/or/dirvote.c
+++ b/src/or/dirvote.c
@@ -64,7 +64,7 @@ STATIC char *
 format_networkstatus_vote(crypto_pk_t *private_signing_key,
                           networkstatus_t *v3_ns)
 {
-  smartlist_t *chunks;
+  smartlist_t *chunks = smartlist_new();
   const char *client_versions = NULL, *server_versions = NULL;
   char fingerprint[FINGERPRINT_LEN+1];
   char digest[DIGEST_LEN];
@@ -98,7 +98,6 @@ format_networkstatus_vote(crypto_pk_t *private_signing_key,
     server_versions_line = tor_strdup("");
   }
 
-  chunks = smartlist_new();
   {
     char published[ISO_TIME_LEN+1];
     char va[ISO_TIME_LEN+1];
@@ -230,10 +229,9 @@ format_networkstatus_vote(crypto_pk_t *private_signing_key,
  done:
   tor_free(client_versions_line);
   tor_free(server_versions_line);
-  if (chunks) {
-    SMARTLIST_FOREACH(chunks, char *, cp, tor_free(cp));
-    smartlist_free(chunks);
-  }
+
+  SMARTLIST_FOREACH(chunks, char *, cp, tor_free(cp));
+  smartlist_free(chunks);
   return status;
 }
 
@@ -2275,8 +2273,11 @@ networkstatus_add_detached_signatures(networkstatus_t *target,
     if (!sig->good_signature && !sig->bad_signature) {
       cert = authority_cert_get_by_digests(sig->identity_digest,
                                            sig->signing_key_digest);
-      if (cert)
-        networkstatus_check_document_signature(target, sig, cert);
+      if (cert) {
+        /* Not checking the return value here, since we are going to look
+         * at the status of sig->good_signature in a moment. */
+        (void) networkstatus_check_document_signature(target, sig, cert);
+      }
     }
 
     /* If this signature is good, or we don't have any signature yet,
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index 629e42cdf2..0633c35fbb 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -1446,10 +1446,7 @@ rend_service_introduce(origin_circuit_t *circuit, const uint8_t *request,
   memwipe(hexcookie, 0, sizeof(hexcookie));
 
   /* Free the parsed cell */
-  if (parsed_req) {
-    rend_service_free_intro(parsed_req);
-    parsed_req = NULL;
-  }
+  rend_service_free_intro(parsed_req);
 
   /* Free rp if we must */
   if (need_rp_free) extend_info_free(rp);
@@ -1539,7 +1536,6 @@ void
 rend_service_free_intro(rend_intro_cell_t *request)
 {
   if (!request) {
-    log_info(LD_BUG, "rend_service_free_intro() called with NULL request!");
     return;
   }
 
@@ -1648,7 +1644,7 @@ rend_service_begin_parse_intro(const uint8_t *request,
   goto done;
 
  err:
-  if (rv) rend_service_free_intro(rv);
+  rend_service_free_intro(rv);
   rv = NULL;
   if (err_msg_out && !err_msg) {
     tor_asprintf(&err_msg,
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 12ed71d01e..14451c0cd8 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -1809,7 +1809,7 @@ scale_array_elements_to_u64(u64_dbl_t *entries, int n_entries,
   double scale_factor;
   int i;
   /* big, but far away from overflowing an int64_t */
-#define SCALE_TO_U64_MAX (INT64_MAX / 4)
+#define SCALE_TO_U64_MAX ((int64_t) (INT64_MAX / 4))
 
   for (i = 0; i < n_entries; ++i)
     total += entries[i].dbl;
diff --git a/src/test/test_entrynodes.c b/src/test/test_entrynodes.c
index fbb7625f84..fab82c3446 100644
--- a/src/test/test_entrynodes.c
+++ b/src/test/test_entrynodes.c
@@ -431,6 +431,7 @@ test_entry_guards_parse_state_simple(void *arg)
  done:
   state_lines_free(entry_state_lines);
   or_state_free(state);
+  tor_free(msg);
 }
 
 /** Similar to test_entry_guards_parse_state_simple() but aims to test
@@ -515,6 +516,7 @@ test_entry_guards_parse_state_pathbias(void *arg)
  done:
   or_state_free(state);
   state_lines_free(entry_state_lines);
+  tor_free(msg);
 }
 
 /* Simple test of entry_guards_set_from_config() by specifying a
diff --git a/src/tools/tor-gencert.c b/src/tools/tor-gencert.c
index e799df5cad..fae26ef956 100644
--- a/src/tools/tor-gencert.c
+++ b/src/tools/tor-gencert.c
@@ -134,18 +134,30 @@ parse_commandline(int argc, char **argv)
         fprintf(stderr, "No argument to -i\n");
         return 1;
       }
+      if (identity_key_file) {
+        fprintf(stderr, "Duplicate values for -i\n");
+        return -1;
+      }
       identity_key_file = tor_strdup(argv[++i]);
     } else if (!strcmp(argv[i], "-s")) {
       if (i+1>=argc) {
         fprintf(stderr, "No argument to -s\n");
         return 1;
       }
+      if (signing_key_file) {
+        fprintf(stderr, "Duplicate values for -s\n");
+        return -1;
+      }
       signing_key_file = tor_strdup(argv[++i]);
     } else if (!strcmp(argv[i], "-c")) {
       if (i+1>=argc) {
         fprintf(stderr, "No argument to -c\n");
         return 1;
       }
+      if (certificate_file) {
+        fprintf(stderr, "Duplicate values for -c\n");
+        return -1;
+      }
       certificate_file = tor_strdup(argv[++i]);
     } else if (!strcmp(argv[i], "-m")) {
       if (i+1>=argc) {