Merge branch 'maint-0.2.9' into release-0.2.9

17 files changed, 118 insertions, 21 deletions

diff --git a/changes/bug28525 b/changes/bug28525
new file mode 100644
index 0000000000..988ffb2192
--- /dev/null
+++ b/changes/bug28525
@@ -0,0 +1,7 @@
+  o Minor features (address selection):
+    - Make Tor aware of the RFC 6598 (Carrier Grade NAT) IP range, which is the
+      subnet 100.64.0.0/10. This is deployed by many ISPs as an alternative to
+      RFC 1918 that does not break existing internal networks. This patch fixes
+      security issues caused by RFC 6518 by blocking control ports on these
+      addresses and warns users if client ports or ExtORPorts are listening on
+      a RFC 6598 address. Closes ticket 28525. Patch by Neel Chauhan.
diff --git a/changes/bug30041 b/changes/bug30041
new file mode 100644
index 0000000000..801c8f67ac
--- /dev/null
+++ b/changes/bug30041
@@ -0,0 +1,5 @@
+  o Minor bugfixes (hardening):
+    - Verify in more places that we are not about to create a buffer
+      with more than INT_MAX bytes, to avoid possible OOB access in the event
+      of bugs.  Fixes bug 30041; bugfix on 0.2.0.16.  Found and fixed by
+      Tobias Stoeckmann.
diff --git a/changes/bug30561 b/changes/bug30561
new file mode 100644
index 0000000000..afb3f02c62
--- /dev/null
+++ b/changes/bug30561
@@ -0,0 +1,6 @@
+  o Minor bugfixes (portability):
+    - Avoid crashing in our tor_vasprintf() implementation on systems that
+      define neither vasprintf() nor _vscprintf(). (This bug has been here
+      long enough that we question whether people are running Tor on such
+      systems, but we're applying the fix out of caution.) Fixes bug 30561;
+      bugfix on 0.2.8.2-alpha. Found and fixed by Tobias Stoeckmann.
diff --git a/changes/bug30781 b/changes/bug30781
new file mode 100644
index 0000000000..7c7adf470e
--- /dev/null
+++ b/changes/bug30781
@@ -0,0 +1,4 @@
+  o Minor bugfixes (directory authorities):
+    - Stop crashing after parsing an unknown descriptor purpose annotation.
+      We think this bug can only be triggered by modifying a local file.
+      Fixes bug 30781; bugfix on 0.2.0.8-alpha.
diff --git a/changes/bug31343 b/changes/bug31343
new file mode 100644
index 0000000000..17a8057ead
--- /dev/null
+++ b/changes/bug31343
@@ -0,0 +1,9 @@
+  o Minor bugfixes (compilation):
+    - Avoid using labs() on time_t, which can cause compilation warnings
+      on 64-bit Windows builds.  Fixes bug 31343; bugfix on 0.2.4.4-alpha.
+
+  o Minor bugfixes (clock skew detection):
+    - Don't believe clock skew results from NETINFO cells that appear to
+      arrive before the VERSIONS cells they are responding to were sent.
+      Previously, we would accept them up to 3 minutes "in the past".
+      Fixes bug 31343; bugfix on 0.2.4.4-alpha.
diff --git a/changes/ticket31374 b/changes/ticket31374
new file mode 100644
index 0000000000..e8eef9cd49
--- /dev/null
+++ b/changes/ticket31374
@@ -0,0 +1,4 @@
+  o Minor bugfixes (compilation warning):
+    - Fix a compilation warning on Windows about casting a function
+      pointer for GetTickCount64(). Fixes bug 31374; bugfix on
+      0.2.9.1-alpha.
diff --git a/src/common/address.c b/src/common/address.c
index 794345a138..71d3805386 100644
--- a/src/common/address.c
+++ b/src/common/address.c
@@ -348,9 +348,18 @@ tor_addr_lookup(const char *name, uint16_t family, tor_addr_t *addr)
   }
 }
 
-/** Return true iff <b>ip</b> is an IP reserved to localhost or local networks
- * in RFC1918 or RFC4193 or RFC4291. (fec0::/10, deprecated by RFC3879, is
- * also treated as internal for now.)
+/** Return true iff <b>ip</b> is an IP reserved to localhost or local networks.
+ *
+ * If <b>ip</b> is in RFC1918 or RFC4193 or RFC4291, we will return true.
+ * (fec0::/10, deprecated by RFC3879, is also treated as internal for now
+ * and will return true.)
+ *
+ * If <b>ip</b> is 0.0.0.0 or 100.64.0.0/10 (RFC6598), we will act as:
+ *  - Internal if <b>for_listening</b> is 0, as these addresses are not
+ *    routable on the internet and we won't be publicly accessible to clients.
+ *  - External if <b>for_listening</b> is 1, as clients could connect to us
+ *    from the internet (in the case of 0.0.0.0) or a service provider's
+ *    internal network (in the case of RFC6598).
  */
 int
 tor_addr_is_internal_(const tor_addr_t *addr, int for_listening,
@@ -398,11 +407,13 @@ tor_addr_is_internal_(const tor_addr_t *addr, int for_listening,
 
     return 0;
   } else if (v_family == AF_INET) {
-    if (for_listening && !iph4) /* special case for binding to 0.0.0.0 */
+    /* special case for binding to 0.0.0.0 or 100.64/10 (RFC6598) */
+    if (for_listening && (!iph4 || ((iph4 & 0xffc00000) == 0x64400000)))
       return 0;
     if (((iph4 & 0xff000000) == 0x0a000000) || /*       10/8 */
         ((iph4 & 0xff000000) == 0x00000000) || /*        0/8 */
         ((iph4 & 0xff000000) == 0x7f000000) || /*      127/8 */
+        ((iph4 & 0xffc00000) == 0x64400000) || /*  100.64/10 */
         ((iph4 & 0xffff0000) == 0xa9fe0000) || /* 169.254/16 */
         ((iph4 & 0xfff00000) == 0xac100000) || /*  172.16/12 */
         ((iph4 & 0xffff0000) == 0xc0a80000))   /* 192.168/16 */
diff --git a/src/common/compat.c b/src/common/compat.c
index 9758751122..ee3bf0fd50 100644
--- a/src/common/compat.c
+++ b/src/common/compat.c
@@ -554,14 +554,24 @@ tor_vasprintf(char **strp, const char *fmt, va_list args)
    * characters we need.  We give it a try on a short buffer first, since
    * it might be nice to avoid the second vsnprintf call.
    */
+  /* XXXX This code spent a number of years broken (see bug 30651). It is
+   * possible that no Tor users actually run on systems without vasprintf() or
+   * _vscprintf(). If so, we should consider removing this code. */
   char buf[128];
   int len, r;
   va_list tmp_args;
   va_copy(tmp_args, args);
-  /* vsnprintf() was properly checked but tor_vsnprintf() available so
-   * why not use it? */
-  len = tor_vsnprintf(buf, sizeof(buf), fmt, tmp_args);
+  /* Use vsnprintf to retrieve needed length.  tor_vsnprintf() is not an
+   * option here because it will simply return -1 if buf is not large enough
+   * to hold the complete string.
+   */
+  len = vsnprintf(buf, sizeof(buf), fmt, tmp_args);
   va_end(tmp_args);
+  buf[sizeof(buf) - 1] = '\0';
+  if (len < 0) {
+    *strp = NULL;
+    return -1;
+  }
   if (len < (int)sizeof(buf)) {
     *strp = tor_strdup(buf);
     return len;
@@ -3543,4 +3553,3 @@ tor_get_avail_disk_space(const char *path)
   return -1;
 #endif
 }
-
diff --git a/src/common/compat_time.c b/src/common/compat_time.c
index d044bbe1d7..52da609db8 100644
--- a/src/common/compat_time.c
+++ b/src/common/compat_time.c
@@ -443,7 +443,7 @@ monotime_init_internal(void)
 
   HANDLE h = load_windows_system_library(TEXT("kernel32.dll"));
   if (h) {
-    GetTickCount64_fn = (GetTickCount64_fn_t)
+    GetTickCount64_fn = (GetTickCount64_fn_t) (void(*)(void))
       GetProcAddress(h, "GetTickCount64");
   }
   // FreeLibrary(h) ?
@@ -654,4 +654,3 @@ monotime_coarse_absolute_msec(void)
   return monotime_coarse_absolute_nsec() / ONE_MILLION;
 }
 #endif
-
diff --git a/src/or/buffers.c b/src/or/buffers.c
index 89382d1d8e..b36e4ab509 100644
--- a/src/or/buffers.c
+++ b/src/or/buffers.c
@@ -1034,6 +1034,7 @@ buf_find_pos_of_char(char ch, buf_pos_t *out)
 static inline int
 buf_pos_inc(buf_pos_t *pos)
 {
+  tor_assert(pos->pos < INT_MAX - 1);
   ++pos->pos;
   if (pos->pos == (off_t)pos->chunk->datalen) {
     if (!pos->chunk->next)
@@ -1925,6 +1926,7 @@ buf_find_offset_of_char(buf_t *buf, char ch)
 {
   chunk_t *chunk;
   off_t offset = 0;
+  tor_assert(buf->datalen < INT_MAX);
   for (chunk = buf->head; chunk; chunk = chunk->next) {
     char *cp = memchr(chunk->data, ch, chunk->datalen);
     if (cp)
@@ -2044,6 +2046,7 @@ assert_buf_ok(buf_t *buf)
     for (ch = buf->head; ch; ch = ch->next) {
       total += ch->datalen;
       tor_assert(ch->datalen <= ch->memlen);
+      tor_assert(ch->datalen < INT_MAX);
       tor_assert(ch->data >= &ch->mem[0]);
       tor_assert(ch->data <= &ch->mem[0]+ch->memlen);
       if (ch->data == &ch->mem[0]+ch->memlen) {
@@ -2060,4 +2063,3 @@ assert_buf_ok(buf_t *buf)
     tor_assert(buf->datalen == total);
   }
 }
-
diff --git a/src/or/channeltls.c b/src/or/channeltls.c
index 3a352d47fe..d44f719138 100644
--- a/src/or/channeltls.c
+++ b/src/or/channeltls.c
@@ -1584,6 +1584,18 @@ channel_tls_process_versions_cell(var_cell_t *cell, channel_tls_t *chan)
 }
 
 /**
+ * Helper: compute the absolute value of a time_t.
+ *
+ * (we need this because labs() doesn't always work for time_t, since
+ * long can be shorter than time_t.)
+ */
+static inline time_t
+time_abs(time_t val)
+{
+  return (val < 0) ? -val : val;
+}
+
+/**
  * Process a 'netinfo' cell
  *
  * This function is called to handle an incoming NETINFO cell; read and act
@@ -1601,7 +1613,7 @@ channel_tls_process_netinfo_cell(cell_t *cell, channel_tls_t *chan)
   uint8_t n_other_addrs;
   time_t now = time(NULL);
 
-  long apparent_skew = 0;
+  time_t apparent_skew = 0;
   tor_addr_t my_apparent_addr = TOR_ADDR_NULL;
 
   tor_assert(cell);
@@ -1659,7 +1671,11 @@ channel_tls_process_netinfo_cell(cell_t *cell, channel_tls_t *chan)
 
   /* Decode the cell. */
   timestamp = ntohl(get_uint32(cell->payload));
-  if (labs(now - chan->conn->handshake_state->sent_versions_at) < 180) {
+  const time_t sent_versions_at =
+    chan->conn->handshake_state->sent_versions_at;
+  if (now > sent_versions_at && (now - sent_versions_at) < 180) {
+    /* If we have gotten the NETINFO cell reasonably soon after having
+     * sent our VERSIONS cell, maybe we can learn skew information from it. */
     apparent_skew = now - timestamp;
   }
 
@@ -1705,7 +1721,7 @@ channel_tls_process_netinfo_cell(cell_t *cell, channel_tls_t *chan)
   /* Act on apparent skew. */
   /** Warn when we get a netinfo skew with at least this value. */
 #define NETINFO_NOTICE_SKEW 3600
-  if (labs(apparent_skew) > NETINFO_NOTICE_SKEW &&
+  if (time_abs(apparent_skew) > NETINFO_NOTICE_SKEW &&
       router_get_by_id_digest(chan->conn->identity_digest)) {
     int trusted = router_digest_is_trusted_dir(chan->conn->identity_digest);
     clock_skew_warning(TO_CONN(chan->conn), apparent_skew, trusted, LD_GENERAL,
@@ -2182,4 +2198,3 @@ channel_tls_process_authenticate_cell(var_cell_t *cell, channel_tls_t *chan)
 
 #undef ERR
 }
-
diff --git a/src/or/connection.c b/src/or/connection.c
index 791fd95c27..4f636eeb8c 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -3581,6 +3581,10 @@ connection_read_to_buf(connection_t *conn, ssize_t *max_to_read,
     if (conn->linked_conn) {
       result = move_buf_to_buf(conn->inbuf, conn->linked_conn->outbuf,
                                &conn->linked_conn->outbuf_flushlen);
+      if (BUG(result<0)) {
+        log_warn(LD_BUG, "reading from linked connection buffer failed.");
+        return -1;
+      }
     } else {
       result = 0;
     }
diff --git a/src/or/rephist.h b/src/or/rephist.h
index c464b34f7c..303cd74f7a 100644
--- a/src/or/rephist.h
+++ b/src/or/rephist.h
@@ -117,9 +117,7 @@ extern uint32_t rephist_total_num;
 #ifdef TOR_UNIT_TESTS
 extern int onion_handshakes_requested[MAX_ONION_HANDSHAKE_TYPE+1];
 extern int onion_handshakes_assigned[MAX_ONION_HANDSHAKE_TYPE+1];
-typedef struct bw_array_t bw_array_t;
-extern bw_array_t *write_array;
+extern struct bw_array_t *write_array;
 #endif
 
 #endif
-
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index f73ec9baa1..f3b298006c 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -5443,7 +5443,7 @@ int
 router_differences_are_cosmetic(const routerinfo_t *r1, const routerinfo_t *r2)
 {
   time_t r1pub, r2pub;
-  long time_difference;
+  time_t time_difference;
   tor_assert(r1 && r2);
 
   /* r1 should be the one that was published first. */
@@ -5506,7 +5506,9 @@ router_differences_are_cosmetic(const routerinfo_t *r1, const routerinfo_t *r2)
    * give or take some slop? */
   r1pub = r1->cache_info.published_on;
   r2pub = r2->cache_info.published_on;
-  time_difference = labs(r2->uptime - (r1->uptime + (r2pub - r1pub)));
+  time_difference = r2->uptime - (r1->uptime + (r2pub - r1pub));
+  if (time_difference < 0)
+    time_difference = - time_difference;
   if (time_difference > ROUTER_ALLOW_UPTIME_DRIFT &&
       time_difference > r1->uptime * .05 &&
       time_difference > r2->uptime * .05)
@@ -5816,4 +5818,3 @@ refresh_all_country_info(void)
 
   nodelist_refresh_countries();
 }
-
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index 37d2d975fc..f046cc39b4 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -1921,6 +1921,9 @@ router_parse_entry_from_string(const char *s, const char *end,
   if ((tok = find_opt_by_keyword(tokens, A_PURPOSE))) {
     tor_assert(tok->n_args);
     router->purpose = router_purpose_from_string(tok->args[0]);
+    if (router->purpose == ROUTER_PURPOSE_UNKNOWN) {
+      goto err;
+    }
   } else {
     router->purpose = ROUTER_PURPOSE_GENERAL;
   }
diff --git a/src/test/test_addr.c b/src/test/test_addr.c
index be440a0925..5c4b6449cd 100644
--- a/src/test/test_addr.c
+++ b/src/test/test_addr.c
@@ -1063,6 +1063,23 @@ test_addr_make_null(void *data)
   tor_free(zeros);
 }
 
+#define TEST_ADDR_INTERNAL(a, for_listening, rv) STMT_BEGIN \
+    tor_addr_t t; \
+    tt_int_op(tor_inet_pton(AF_INET, a, &t.addr.in_addr), OP_EQ, 1); \
+    t.family = AF_INET; \
+    tt_int_op(tor_addr_is_internal(&t, for_listening), OP_EQ, rv); \
+  STMT_END;
+
+static void
+test_addr_rfc6598(void *arg)
+{
+  (void)arg;
+  TEST_ADDR_INTERNAL("100.64.0.1", 0, 1);
+  TEST_ADDR_INTERNAL("100.64.0.1", 1, 0);
+ done:
+  ;
+}
+
 #define ADDR_LEGACY(name)                                               \
   { #name, test_addr_ ## name , 0, NULL, NULL }
 
@@ -1076,6 +1093,7 @@ struct testcase_t addr_tests[] = {
   { "sockaddr_to_str", test_addr_sockaddr_to_str, 0, NULL, NULL },
   { "is_loopback", test_addr_is_loopback, 0, NULL, NULL },
   { "make_null", test_addr_make_null, 0, NULL, NULL },
+  { "rfc6598", test_addr_rfc6598, 0, NULL, NULL },
   END_OF_TESTCASES
 };
 
diff --git a/src/test/test_relay.c b/src/test/test_relay.c
index 57dcb2406a..d4ebb23382 100644
--- a/src/test/test_relay.c
+++ b/src/test/test_relay.c
@@ -19,6 +19,8 @@
 static or_circuit_t * new_fake_orcirc(channel_t *nchan, channel_t *pchan);
 
 static void test_relay_append_cell_to_circuit_queue(void *arg);
+
+typedef struct bw_array_t bw_array_t;
 uint64_t find_largest_max(bw_array_t *b);
 void commit_max(bw_array_t *b);
 void advance_obs(bw_array_t *b);