Merge branch 'maint-0.4.0' into release-0.4.0

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.

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.

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.

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.

src/core/or/channeltls.c

@@ -1664,7 +1664,19 @@ tor_addr_from_netinfo_addr(tor_addr_t *tor_addr,
 }
 
 /**
- * Process a 'netinfo' cell.
+ * 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
  * on its contents, and set the connection state to "open".
@@ -1679,7 +1691,7 @@ channel_tls_process_netinfo_cell(cell_t *cell, channel_tls_t *chan)
   time_t now = time(NULL);
   const routerinfo_t *me = router_get_my_routerinfo();
 
-  long apparent_skew = 0;
+  time_t apparent_skew = 0;
   tor_addr_t my_apparent_addr = TOR_ADDR_NULL;
   int started_here = 0;
   const char *identity_digest = NULL;
@@ -1765,7 +1777,7 @@ channel_tls_process_netinfo_cell(cell_t *cell, channel_tls_t *chan)
   my_addr_type = netinfo_addr_get_addr_type(my_addr);
   my_addr_len = netinfo_addr_get_len(my_addr);
 
-  if (labs(now - chan->conn->handshake_state->sent_versions_at) < 180) {
+  if ((now - chan->conn->handshake_state->sent_versions_at) < 180) {
     apparent_skew = now - timestamp;
   }
   /* We used to check:
@@ -1842,7 +1854,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 &&
       (started_here ||
        connection_or_digest_is_known_relay(chan->conn->identity_digest))) {
     int trusted = router_digest_is_trusted_dir(chan->conn->identity_digest);

src/feature/dirparse/routerparse.c

@@ -556,6 +556,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;
   }

src/feature/nodelist/routerlist.c

@@ -2856,7 +2856,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. */
@@ -2920,7 +2920,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)

src/lib/string/printf.c

@@ -131,14 +131,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;

src/lib/time/compat_time.c

@@ -519,7 +519,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");
   }
   // We can't call FreeLibrary(h) here, because freeing the handle may