Merge branch 'clang_fixes' into maint-0.2.2

9 files changed, 33 insertions, 10 deletions

diff --git a/changes/dirvote_null_deref b/changes/dirvote_null_deref
new file mode 100644
index 000000000..65dc519f5
--- /dev/null
+++ b/changes/dirvote_null_deref
@@ -0,0 +1,4 @@
+  o Minor bugfixes:
+    - Fix a potential null-pointer dereference while computing a consensus.
+      Bugfix on tor-0.2.0.3-alpha, found with the help of clang's analyzer.
+
diff --git a/changes/mdesc_null_deref b/changes/mdesc_null_deref
new file mode 100644
index 000000000..30f028053
--- /dev/null
+++ b/changes/mdesc_null_deref
@@ -0,0 +1,5 @@
+  o Minor bugfixes:
+    - Avoid a possible null-pointer dereference when rebuilding the mdesc
+      cache without actually having any descriptors to cache. Bugfix on
+      0.2.2.6-alpha. Issue discovered using clang's static analyzer.
+
diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index 0e033a589..2189e0e55 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -559,7 +559,9 @@ circuit_build_times_create_histogram(circuit_build_times_t *cbt,
  * Return the Pareto start-of-curve parameter Xm.
  *
  * Because we are not a true Pareto curve, we compute this as the
- * weighted average of the N=3 most frequent build time bins.
+ * weighted average of the N most frequent build time bins. N is either
+ * 1 if we don't have enough circuit build time data collected, or
+ * determined by the consensus parameter cbtnummodes (default 3).
  */
 static build_time_t
 circuit_build_times_get_xm(circuit_build_times_t *cbt)
@@ -572,6 +574,9 @@ circuit_build_times_get_xm(circuit_build_times_t *cbt)
   int n=0;
   int num_modes = circuit_build_times_default_num_xm_modes();
 
+  tor_assert(nbins > 0);
+  tor_assert(num_modes > 0);
+
   // Only use one mode if < 1000 buildtimes. Not enough data
   // for multiple.
   if (cbt->total_build_times < CBT_NCIRCUITS_TO_OBSERVE)
@@ -579,6 +584,7 @@ circuit_build_times_get_xm(circuit_build_times_t *cbt)
 
   nth_max_bin = (build_time_t*)tor_malloc_zero(num_modes*sizeof(build_time_t));
 
+  /* Determine the N most common build times */
   for (i = 0; i < nbins; i++) {
     if (histogram[i] >= histogram[nth_max_bin[0]]) {
       nth_max_bin[0] = i;
@@ -600,6 +606,10 @@ circuit_build_times_get_xm(circuit_build_times_t *cbt)
              histogram[nth_max_bin[n]]);
   }
 
+  /* The following assert is safe, because we don't get called when we
+   * haven't observed at least CBT_MIN_MIN_CIRCUITS_TO_OBSERVE circuits. */
+  tor_assert(bin_counts > 0);
+
   ret /= bin_counts;
   tor_free(histogram);
   tor_free(nth_max_bin);
diff --git a/src/or/connection.c b/src/or/connection.c
index 6e7bbd5ba..fc2097f9a 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -1240,7 +1240,7 @@ connection_connect(connection_t *conn, const char *address,
 {
   int s, inprogress = 0;
   char addrbuf[256];
-  struct sockaddr *dest_addr = (struct sockaddr*) addrbuf;
+  struct sockaddr *dest_addr;
   socklen_t dest_addr_len;
   or_options_t *options = get_options();
   int protocol_family;
diff --git a/src/or/connection_or.c b/src/or/connection_or.c
index 4b932ec51..c6049d51f 100644
--- a/src/or/connection_or.c
+++ b/src/or/connection_or.c
@@ -1414,9 +1414,8 @@ connection_or_send_netinfo(or_connection_t *conn)
     len = append_address_to_payload(out, &my_addr);
     if (len < 0)
       return -1;
-    out += len;
   } else {
-    *out++ = 0;
+    *out = 0;
   }
 
   connection_or_write_cell_to_buf(&cell, conn);
diff --git a/src/or/control.c b/src/or/control.c
index 9f7739a40..926a46520 100644
--- a/src/or/control.c
+++ b/src/or/control.c
@@ -3875,7 +3875,7 @@ static int bootstrap_problems = 0;
  * information and initial circuits.
  *
  * <b>status</b> is the new status, that is, what task we will be doing
- * next. <b>percent</b> is zero if we just started this task, else it
+ * next. <b>progress</b> is zero if we just started this task, else it
  * represents progress on the task. */
 void
 control_event_bootstrap(bootstrap_status_t status, int progress)
@@ -3931,6 +3931,9 @@ control_event_bootstrap_problem(const char *warn, int reason)
   char buf[BOOTSTRAP_MSG_LEN];
   const char *recommendation = "ignore";
 
+  /* bootstrap_percent must not be in "undefined" state here. */
+  tor_assert(status >= 0);
+
   if (bootstrap_percent == 100)
     return; /* already bootstrapped; nothing to be done here. */
 
diff --git a/src/or/dirvote.c b/src/or/dirvote.c
index db2eaf0f4..750c649f5 100644
--- a/src/or/dirvote.c
+++ b/src/or/dirvote.c
@@ -444,9 +444,9 @@ compute_routerstatus_consensus(smartlist_t *votes, int consensus_method,
     if (cur && !compare_vote_rs(cur, rs)) {
       ++cur_n;
     } else {
-      if (cur_n > most_n ||
-          (cur && cur_n == most_n &&
-           cur->status.published_on > most_published)) {
+      if (cur && (cur_n > most_n ||
+                  (cur_n == most_n &&
+                   cur->status.published_on > most_published))) {
         most = cur;
         most_n = cur_n;
         most_published = cur->status.published_on;
diff --git a/src/or/microdesc.c b/src/or/microdesc.c
index 73d228500..5740c40d5 100644
--- a/src/or/microdesc.c
+++ b/src/or/microdesc.c
@@ -423,7 +423,7 @@ microdesc_cache_rebuild(microdesc_cache_t *cache, int force)
   cache->journal_len = 0;
   cache->bytes_dropped = 0;
 
-  new_size = (int)cache->cache_content->size;
+  new_size = cache->cache_content ? (int)cache->cache_content->size : 0;
   log_info(LD_DIR, "Done rebuilding microdesc cache. "
            "Saved %d bytes; %d still used.",
            orig_size-new_size, new_size);
diff --git a/src/or/or.h b/src/or/or.h
index 7d354c8fe..f693ad908 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -3203,7 +3203,9 @@ typedef enum buildtimeout_set_event_t {
  */
 #define CONN_LOG_PROTECT(conn, stmt)                                    \
   STMT_BEGIN                                                            \
-    int _log_conn_is_control = (conn && conn->type == CONN_TYPE_CONTROL); \
+    int _log_conn_is_control;                                           \
+    tor_assert(conn);                                                   \
+    _log_conn_is_control = (conn->type == CONN_TYPE_CONTROL);           \
     if (_log_conn_is_control)                                           \
       disable_control_logging();                                        \
   STMT_BEGIN stmt; STMT_END;                                            \