Merge branch 'maint-0.2.2' into release-0.2.2

5 files changed, 38 insertions, 5 deletions

diff --git a/changes/bug5760 b/changes/bug5760
new file mode 100644
index 0000000000..a26407b588
--- /dev/null
+++ b/changes/bug5760
@@ -0,0 +1,3 @@
+  o Major bugfixes:
+    - End AUTHCHALLENGE error response messages with a CRLF. Fixes bug 5760;
+      bugfix on 0.2.3.16-alpha, and backported to maint-0.2.2
diff --git a/changes/bug5786_range b/changes/bug5786_range
new file mode 100644
index 0000000000..40ac4d2467
--- /dev/null
+++ b/changes/bug5786_range
@@ -0,0 +1,8 @@
+  o Minor bugfixes:
+    - Make our number-parsing functions always treat too-large values
+      as an error, even when those values exceed the width of the
+      underlying type. Previously, if the caller provided these
+      functions with minima or maxima set to the extreme values of the
+      underlying integer type, these functions would return those
+      values on overflow rather than treating overflow as an error.
+      Fix for part of bug 5786; bugfix on Tor 0.0.9.
\ No newline at end of file
diff --git a/src/common/util.c b/src/common/util.c
index e3cd154b93..7d2fc4dea8 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -764,6 +764,9 @@ tor_digest256_is_zero(const char *digest)
 /* Helper: common code to check whether the result of a strtol or strtoul or
  * strtoll is correct. */
 #define CHECK_STRTOX_RESULT()                           \
+  /* Did an overflow occur? */                          \
+  if (errno == ERANGE)                                  \
+    goto err;                                           \
   /* Was at least one character converted? */           \
   if (endptr == s)                                      \
     goto err;                                           \
@@ -800,6 +803,7 @@ tor_parse_long(const char *s, int base, long min, long max,
   char *endptr;
   long r;
 
+  errno = 0;
   r = strtol(s, &endptr, base);
   CHECK_STRTOX_RESULT();
 }
@@ -812,6 +816,7 @@ tor_parse_ulong(const char *s, int base, unsigned long min,
   char *endptr;
   unsigned long r;
 
+  errno = 0;
   r = strtoul(s, &endptr, base);
   CHECK_STRTOX_RESULT();
 }
@@ -823,6 +828,7 @@ tor_parse_double(const char *s, double min, double max, int *ok, char **next)
   char *endptr;
   double r;
 
+  errno = 0;
   r = strtod(s, &endptr);
   CHECK_STRTOX_RESULT();
 }
@@ -836,6 +842,7 @@ tor_parse_uint64(const char *s, int base, uint64_t min,
   char *endptr;
   uint64_t r;
 
+  errno = 0;
 #ifdef HAVE_STRTOULL
   r = (uint64_t)strtoull(s, &endptr, base);
 #elif defined(MS_WINDOWS)
diff --git a/src/or/control.c b/src/or/control.c
index ddfc80e8fd..5ffe63b724 100644
--- a/src/or/control.c
+++ b/src/or/control.c
@@ -2839,13 +2839,13 @@ handle_control_authchallenge(control_connection_t *conn, uint32_t len,
     cp += strlen("SAFECOOKIE");
   } else {
     connection_write_str_to_buf("513 AUTHCHALLENGE only supports SAFECOOKIE "
-                                "authentication", conn);
+                                "authentication\r\n", conn);
     connection_mark_for_close(TO_CONN(conn));
     return -1;
   }
 
   if (!authentication_cookie_is_set) {
-    connection_write_str_to_buf("515 Cookie authentication is disabled", conn);
+    connection_write_str_to_buf("515 Cookie authentication is disabled\r\n", conn);
     connection_mark_for_close(TO_CONN(conn));
     return -1;
   }
@@ -2856,7 +2856,7 @@ handle_control_authchallenge(control_connection_t *conn, uint32_t len,
       decode_escaped_string(cp, len - (cp - body),
                             &client_nonce, &client_nonce_len);
     if (newcp == NULL) {
-      connection_write_str_to_buf("513 Invalid quoted client nonce",
+      connection_write_str_to_buf("513 Invalid quoted client nonce\r\n",
                                   conn);
       connection_mark_for_close(TO_CONN(conn));
       return -1;
@@ -2870,7 +2870,7 @@ handle_control_authchallenge(control_connection_t *conn, uint32_t len,
 
     if (base16_decode(client_nonce, client_nonce_len,
                       cp, client_nonce_encoded_len) < 0) {
-      connection_write_str_to_buf("513 Invalid base16 client nonce",
+      connection_write_str_to_buf("513 Invalid base16 client nonce\r\n",
                                   conn);
       connection_mark_for_close(TO_CONN(conn));
       return -1;
@@ -2882,7 +2882,7 @@ handle_control_authchallenge(control_connection_t *conn, uint32_t len,
   cp += strspn(cp, " \t\n\r");
   if (*cp != '\0' ||
       cp != body + len) {
-    connection_write_str_to_buf("513 Junk at end of AUTHCHALLENGE command",
+    connection_write_str_to_buf("513 Junk at end of AUTHCHALLENGE command\r\n",
                                 conn);
     connection_mark_for_close(TO_CONN(conn));
     tor_free(client_nonce);
diff --git a/src/test/test_util.c b/src/test/test_util.c
index 23cd059cf7..ee745c5cf0 100644
--- a/src/test/test_util.c
+++ b/src/test/test_util.c
@@ -283,6 +283,21 @@ test_util_strmisc(void)
   test_assert(i == 1);
   }
 
+  {
+    /* Test tor_parse_* where we overflow/underflow the underlying type. */
+    /* This string should overflow 64-bit ints. */
+#define TOOBIG "100000000000000000000000000"
+    test_eq(0L, tor_parse_long(TOOBIG, 10, LONG_MIN, LONG_MAX, &i, NULL));
+    test_eq(i, 0);
+    test_eq(0L, tor_parse_long("-"TOOBIG, 10, LONG_MIN, LONG_MAX, &i, NULL));
+    test_eq(i, 0);
+    test_eq(0UL, tor_parse_ulong(TOOBIG, 10, 0, ULONG_MAX, &i, NULL));
+    test_eq(i, 0);
+    test_eq(U64_LITERAL(0), tor_parse_uint64(TOOBIG, 10,
+                                             0, UINT64_MAX, &i, NULL));
+    test_eq(i, 0);
+  }
+
   /* Test failing snprintf cases */
   test_eq(-1, tor_snprintf(buf, 0, "Foo"));
   test_eq(-1, tor_snprintf(buf, 2, "Foo"));