[tor-commits] [tor/master] hs: Fix an underflow in rend_service_intro_has_opened()

[nickm at torproject.org]

commit 8573d994706c16628bb60e0aabdb9a5190beff13
Author: David Goulet <dgoulet at torproject.org>
Date:   Wed Feb 1 11:22:17 2017 -0500

    hs: Fix an underflow in rend_service_intro_has_opened()
    
    In rend_service_intro_has_opened(), this is subject to a possible underflow
    because of how the if() casts the results. In the case where the expiring
    nodes list length is bigger than the number of IP circuits, we end up in the
    following situation where the result will be cast to an unsigned int. For
    instance, "5 - 6" is actually a BIG number.
    
    Ultimately leading to closing IP circuits in a non stop loop.
    
    Partially fixes #21302.
    
    Signed-off-by: David Goulet <dgoulet at torproject.org>
---
 src/or/rendservice.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index 5514de6..0db81ae 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -3227,6 +3227,7 @@ rend_service_intro_has_opened(origin_circuit_t *circuit)
   rend_service_t *service;
   char buf[RELAY_PAYLOAD_SIZE];
   char serviceid[REND_SERVICE_ID_LEN_BASE32+1];
+  unsigned int expiring_nodes_len, num_ip_circuits, valid_ip_circuits = 0;
   int reason = END_CIRC_REASON_TORPROTOCOL;
   const char *rend_pk_digest;
 
@@ -3248,13 +3249,22 @@ rend_service_intro_has_opened(origin_circuit_t *circuit)
     goto err;
   }
 
+  /* Take the current amount of expiring nodes and the current amount of IP
+   * circuits and compute how many valid IP circuits we have. */
+  expiring_nodes_len = (unsigned int) smartlist_len(service->expiring_nodes);
+  num_ip_circuits = count_intro_point_circuits(service);
+  /* Let's avoid an underflow. The valid_ip_circuits is initialized to 0 in
+   * case this condition turns out false because it means that all circuits
+   * are expiring so we need to keep this circuit. */
+  if (num_ip_circuits > expiring_nodes_len) {
+    valid_ip_circuits = num_ip_circuits - expiring_nodes_len;
+  }
+
   /* If we already have enough introduction circuits for this service,
    * redefine this one as a general circuit or close it, depending.
-   * Substract the amount of expiring nodes here since the circuits are
+   * Substract the amount of expiring nodes here because the circuits are
    * still opened. */
-  if ((count_intro_point_circuits(service) -
-       smartlist_len(service->expiring_nodes)) >
-      service->n_intro_points_wanted) {
+  if (valid_ip_circuits > service->n_intro_points_wanted) {
     const or_options_t *options = get_options();
     /* Remove the intro point associated with this circuit, it's being
      * repurposed or closed thus cleanup memory. */