General tweaks and fixes for Nick's comments.

* Add changes/ files.
* Edit the tor-fw-helper manpage.
* Fix check-spaces.
* Add prototype for get_list_of_ports_to_forward().
* Fix tor_parse_long() TCP port range.
* Improve doc. of tor_check_port_forwarding().
* Check for overflows in tor_check_port_forwarding().
* Demote successful port forwarding to LOG_INFO.

Conflicts:
	src/common/address.c
	src/or/circuitbuild.c

5 files changed, 47 insertions, 22 deletions

diff --git a/src/common/address.c b/src/common/address.c
index e5862be1e..ac45cba95 100644
--- a/src/common/address.c
+++ b/src/common/address.c
@@ -1697,7 +1697,6 @@ tor_addr_hostname_is_local(const char *name)
     !strcasecmpend(name, ".local");
 }
 
-
 /** Return a newly allocated tor_addr_port_t with <b>addr</b> and
     <b>port</b> filled in. */
 tor_addr_port_t *
diff --git a/src/common/util.c b/src/common/util.c
index b1a05b576..25ddcc1fb 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -4676,10 +4676,10 @@ handle_fw_helper_line(const char *line)
              message_for_log ? message_for_log : "",
              internal_port);
   } else {
-    log_notice(LD_GENERAL,
-               "Tor successfully forwarded TCP port '%s' to '%s'%s.",
-               external_port, internal_port,
-               message_for_log ? message_for_log : "");
+    log_info(LD_GENERAL,
+             "Tor successfully forwarded TCP port '%s' to '%s'%s.",
+             external_port, internal_port,
+             message_for_log ? message_for_log : "");
   }
 
   goto done;
@@ -4723,7 +4723,9 @@ handle_fw_helper_output(process_handle_t *process_handle)
 }
 
 /** Spawn tor-fw-helper and ask it to forward the ports in
- *  <b>ports_to_forward</b>. */
+ *  <b>ports_to_forward</b>. <b>ports_to_forward</b> contains strings
+ *  of the form "<external port>:<internal port>", which is the format
+ *  that tor-fw-helper expects. */
 void
 tor_check_port_forwarding(const char *filename,
                           smartlist_t *ports_to_forward,
@@ -4748,17 +4750,35 @@ tor_check_port_forwarding(const char *filename,
   /* Start the child, if it is not already running */
   if ((!child_handle || child_handle->status != PROCESS_STATUS_RUNNING) &&
       time_to_run_helper < now) {
-    /* tor-fw-helper cli looks like this: tor_fw_helper -p :5555 -p 4555:1111 */
+    /*tor-fw-helper cli looks like this: tor_fw_helper -p :5555 -p 4555:1111 */
     const char **argv; /* cli arguments */
-    /* Number of cli arguments: one for the filename, two for each
-       smartlist element (one for "-p" and one for the ports), and one
-       for the final NULL. */
-    int args_n = 1 + 2*smartlist_len(ports_to_forward) + 1;
+    int args_n, status;
     int argv_index = 0; /* index inside 'argv' */
-    int status;
 
     tor_assert(smartlist_len(ports_to_forward) > 0);
 
+    /* check for overflow during 'argv' allocation:
+       (len(ports_to_forward)*2 + 2)*sizeof(char*) > SIZE_MAX ==
+       len(ports_to_forward) > (((SIZE_MAX/sizeof(char*)) - 2)/2) */
+    if ((size_t) smartlist_len(ports_to_forward) >
+        (((SIZE_MAX/sizeof(char*)) - 2)/2)) {
+      log_warn(LD_GENERAL,
+               "Overflow during argv allocation. This shouldn't happen.");
+      return;
+    }
+    /* check for overflow during 'argv_index' increase:
+       ((len(ports_to_forward)*2 + 2) > INT_MAX) ==
+       len(ports_to_forward) > (INT_MAX - 2)/2 */
+    if (smartlist_len(ports_to_forward) > (INT_MAX - 2)/2) {
+      log_warn(LD_GENERAL,
+               "Overflow during argv_index increase. This shouldn't happen.");
+      return;
+    }
+
+    /* Calculate number of cli arguments: one for the filename, two
+       for each smartlist element (one for "-p" and one for the
+       ports), and one for the final NULL. */
+    args_n = 1 + 2*smartlist_len(ports_to_forward) + 1;
     argv = tor_malloc_zero(sizeof(char*)*args_n);
 
     argv[argv_index++] = filename;
diff --git a/src/or/config.c b/src/or/config.c
index ad422efcd..a4af22a37 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -7275,10 +7275,12 @@ get_list_of_ports_to_forward(void)
     smartlist_add_asprintf(ports_to_forward, "%d:%d", port, port);
 
   /* Get ports of transport proxies */
-  smartlist_t *transport_ports = get_transport_proxy_ports();
-  if (transport_ports) {
-    smartlist_add_all(ports_to_forward, transport_ports);
-    smartlist_free(transport_ports);
+  {
+    smartlist_t *transport_ports = get_transport_proxy_ports();
+    if (transport_ports) {
+      smartlist_add_all(ports_to_forward, transport_ports);
+      smartlist_free(transport_ports);
+    }
   }
 
   if (!smartlist_len(ports_to_forward)) {
diff --git a/src/or/config.h b/src/or/config.h
index dd76edcf1..d20796584 100644
--- a/src/or/config.h
+++ b/src/or/config.h
@@ -82,6 +82,8 @@ void save_transport_to_state(const char *transport_name,
                              const tor_addr_t *addr, uint16_t port);
 char *get_stored_bindaddr_for_server_transport(const char *transport);
 
+smartlist_t *get_list_of_ports_to_forward(void);
+
 int getinfo_helper_config(control_connection_t *conn,
                           const char *question, char **answer,
                           const char **errmsg);
diff --git a/src/tools/tor-fw-helper/tor-fw-helper.c b/src/tools/tor-fw-helper/tor-fw-helper.c
index 32633542c..d02b75791 100644
--- a/src/tools/tor-fw-helper/tor-fw-helper.c
+++ b/src/tools/tor-fw-helper/tor-fw-helper.c
@@ -249,10 +249,11 @@ tor_fw_add_ports(tor_fw_options_t *tor_fw_options,
                 (const char *) backends->backend_ops[i].name);
       }
 
-      r = backends->backend_ops[i].add_tcp_mapping(port_to_forward->internal_port,
-                                                   port_to_forward->external_port,
-                                                   tor_fw_options->verbose,
-                                                   backends->backend_state[i]);
+      r =
+       backends->backend_ops[i].add_tcp_mapping(port_to_forward->internal_port,
+                                                port_to_forward->external_port,
+                                                tor_fw_options->verbose,
+                                                backends->backend_state[i]);
       if (r == 0) { /* backend success */
         tor_fw_helper_report_port_fw_success(port_to_forward->internal_port,
                                              port_to_forward->external_port,
@@ -326,13 +327,13 @@ parse_port(const char *arg)
     goto err;
 
   port_str = smartlist_get(sl, 0); /* macroify ? */
-  port = (int)tor_parse_long(port_str, 10, 1, 65536, &ok, NULL);
+  port = (int)tor_parse_long(port_str, 10, 1, 65535, &ok, NULL);
   if (!ok && strlen(port_str)) /* ":1555" is valid */
     goto err;
   port_to_forward->external_port = port;
 
   port_str = smartlist_get(sl, 1);
-  port = (int)tor_parse_long(port_str, 10, 1, 65536, &ok, NULL);
+  port = (int)tor_parse_long(port_str, 10, 1, 65535, &ok, NULL);
   if (!ok)
     goto err;
   port_to_forward->internal_port = port;
@@ -507,3 +508,4 @@ main(int argc, char **argv)
 
   exit(r);
 }
+