[tor-commits] [tor/main] net: Reject invalid characters in port ranges

[dgoulet at torproject.org]

commit 6ada3be8f17ebc81352a3e44c4a92f8adff6bfee
Author: c <c at chroniko.jp>
Date:   Fri Oct 9 03:10:24 2020 +0000

    net: Reject invalid characters in port ranges
    
    Fixes issue #22469 where port strings such as '0x00' get accepted, not
    because the string gets converted to hex, but because the string is
    silently truncated past the invalid character 'x'. This also causes
    issues for strings such as '0x01-0x02' which look like a hex port range,
    but in reality gets truncated to '0', which is definitely not what a
    user intends.
    
    Warn and reject such port strings as invalid.
    
    Also, since we're throwing that "malformed port" warning a lot in the
    function, wrap it up in a nice goto.
    
    Fixes #22469
---
 src/lib/net/address.c   | 22 +++++++++++-----------
 src/test/test_address.c | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 11 deletions(-)

diff --git a/src/lib/net/address.c b/src/lib/net/address.c
index 26b155bc4c..085eb8c458 100644
--- a/src/lib/net/address.c
+++ b/src/lib/net/address.c
@@ -2005,20 +2005,15 @@ parse_port_range(const char *port, uint16_t *port_min_out,
     char *endptr = NULL;
     port_min = (int)tor_parse_long(port, 10, 0, 65535, &ok, &endptr);
     if (!ok) {
-      log_warn(LD_GENERAL,
-               "Malformed port %s on address range; rejecting.",
-               escaped(port));
-      return -1;
-    } else if (endptr && *endptr == '-') {
+      goto malformed_port;
+    } else if (endptr && *endptr != '\0') {
+      if (*endptr != '-')
+        goto malformed_port;
       port = endptr+1;
       endptr = NULL;
       port_max = (int)tor_parse_long(port, 10, 1, 65535, &ok, &endptr);
-      if (!ok) {
-        log_warn(LD_GENERAL,
-                 "Malformed port %s on address range; rejecting.",
-                 escaped(port));
-        return -1;
-      }
+      if (!ok)
+        goto malformed_port;
     } else {
       port_max = port_min;
     }
@@ -2037,6 +2032,11 @@ parse_port_range(const char *port, uint16_t *port_min_out,
   *port_max_out = (uint16_t) port_max;
 
   return 0;
+ malformed_port:
+  log_warn(LD_GENERAL,
+           "Malformed port %s on address range; rejecting.",
+           escaped(port));
+  return -1;
 }
 
 /** Given a host-order <b>addr</b>, call tor_inet_ntop() on it
diff --git a/src/test/test_address.c b/src/test/test_address.c
index 9c1415419c..015ca0807c 100644
--- a/src/test/test_address.c
+++ b/src/test/test_address.c
@@ -1326,6 +1326,42 @@ test_address_dirserv_router_addr_private(void *opt_dir_allow_private)
   UNMOCK(get_options);
 }
 
+static void
+test_address_parse_port_range(void *arg)
+{
+  int ret;
+  uint16_t min_out = 0;
+  uint16_t max_out = 0;
+
+  (void) arg;
+
+  /* Invalid. */
+  ret = parse_port_range("0x00", &min_out, &max_out);
+  tt_int_op(ret, OP_EQ, -1);
+  ret = parse_port_range("0x01", &min_out, &max_out);
+  tt_int_op(ret, OP_EQ, -1);
+  ret = parse_port_range("1817161", &min_out, &max_out);
+  tt_int_op(ret, OP_EQ, -1);
+  ret = parse_port_range("65536", &min_out, &max_out);
+  tt_int_op(ret, OP_EQ, -1);
+  ret = parse_port_range("1-65536", &min_out, &max_out);
+  tt_int_op(ret, OP_EQ, -1);
+
+  /* Valid. */
+  ret = parse_port_range("65535", &min_out, &max_out);
+  tt_int_op(ret, OP_EQ, 0);
+  tt_int_op(min_out, OP_EQ, 65535);
+  tt_int_op(max_out, OP_EQ, 65535);
+
+  ret = parse_port_range("1-65535", &min_out, &max_out);
+  tt_int_op(ret, OP_EQ, 0);
+  tt_int_op(min_out, OP_EQ, 1);
+  tt_int_op(max_out, OP_EQ, 65535);
+
+ done:
+  ;
+}
+
 #define ADDRESS_TEST(name, flags) \
   { #name, test_address_ ## name, flags, NULL, NULL }
 #define ADDRESS_TEST_STR_ARG(name, flags, str_arg) \
@@ -1364,5 +1400,6 @@ struct testcase_t address_tests[] = {
   ADDRESS_TEST(tor_node_in_same_network_family, 0),
   ADDRESS_TEST(dirserv_router_addr_private, 0),
   ADDRESS_TEST_STR_ARG(dirserv_router_addr_private, 0, "allow_private"),
+  ADDRESS_TEST(parse_port_range, 0),
   END_OF_TESTCASES
 };