Commit c56bb300 authored by Nick Mathewson's avatar Nick Mathewson 🤹
Browse files

Remove a check in channeltls.c that could never fail. 

We were checking whether a 8-bit length field had overflowed a
503-byte buffer. Unless somebody has found a way to store "504" in a
single byte, it seems unlikely.

Fix for 10313 and 9980. Based on a pach by Jared L Wong. First found
by David Fifield with STACK.
parent 7ef2939e

changes/bug10313

0 → 100644
+8 −0
Original line number Diff line number Diff line 
  o Minor bugfixes:

    - Fixed an erroneous pointer comparison that would have allowed

      compilers to remove a bounds check in channeltls.c. The fix

      was to remove the check entirely, since it was impossible for

      the code to overflow the bounds. Noticed by Jared L

      Wong. Fixes bug 10313 and 9980. Bugfix on 0.2.0.10-alpha.

src/or/channeltls.c

+8 −6
Original line number Diff line number Diff line
@@ -1408,12 +1408,14 @@ channel_tls_process_netinfo_cell(cell_t *cell, channel_tls_t *chan) 
  my_addr_ptr = (uint8_t*) cell->payload + 6;

  end = cell->payload + CELL_PAYLOAD_SIZE;

  cp = cell->payload + 6 + my_addr_len;

  if (cp >= end) {

    log_fn(LOG_PROTOCOL_WARN, LD_OR,

           "Addresses too long in netinfo cell; closing connection.");

    connection_or_close_for_error(chan->conn, 0);

    return;

  } else if (my_addr_type == RESOLVED_TYPE_IPV4 && my_addr_len == 4) {



  /* We used to check:

   *    if (my_addr_len >= CELL_PAYLOAD_SIZE - 6) {

   *

   * This is actually never going to happen, since my_addr_len is at most 255,

   * and CELL_PAYLOAD_LEN - 6 is 503.  So we know that cp is < end. */



  if (my_addr_type == RESOLVED_TYPE_IPV4 && my_addr_len == 4) {

    tor_addr_from_ipv4n(&my_apparent_addr, get_uint32(my_addr_ptr));

  } else if (my_addr_type == RESOLVED_TYPE_IPV6 && my_addr_len == 16) {

    tor_addr_from_ipv6_bytes(&my_apparent_addr, (const char *) my_addr_ptr);