Commit 595303fd authored by Nick Mathewson's avatar Nick Mathewson 🤹
Browse files

Merge remote-tracking branch 'public/bug10363_024_squashed'

parents 84df7d73 6d9c3327

changes/bug10363

0 → 100644
+12 −0
Original line number Diff line number Diff line 
  o Major bugfixes:

    - Fix two instances of possible undefined behavior in channeltls.c

      that could, under unlucky circumstances, have led to a pointer

      overflow. Fixes bug #10363; bugfixes on 0.2.0.10-alpha and

      0.2.3.6-alpha. Reported by "bobnomnom".

    - Fix another possibly undefined pointer operations in tor_memmem

      fallback implementation. Another case of bug #10363; bugfix on

      0.1.1.1-alpha.

    - Fix another possibly undefined pointer operations in the eventdns

      fallback implementation. Another case of bug #10363; bugfix on

      0.1.2.1-alpha.

src/common/compat.c

+14 −6
Original line number Diff line number Diff line
@@ -560,21 +560,29 @@ tor_memmem(const void *_haystack, size_t hlen, 
#else

  /* This isn't as fast as the GLIBC implementation, but it doesn't need to

   * be. */

  const char *p, *end;

  const char *p, *last_possible_start;

  const char *haystack = (const char*)_haystack;

  const char *needle = (const char*)_needle;

  char first;

  tor_assert(nlen);



  if (nlen > hlen)

    return NULL;



  p = haystack;

  end = haystack + hlen;

  /* Last position at which the needle could start. */

  last_possible_start = haystack + hlen - nlen;

  first = *(const char*)needle;

  while ((p = memchr(p, first, end-p))) {

    if (p+nlen > end)

      return NULL;

  while ((p = memchr(p, first, last_possible_start + 1 - p))) {

    if (fast_memeq(p, needle, nlen))

      return p;

    ++p;

    if (++p > last_possible_start) {

      /* This comparison shouldn't be necessary, since if p was previously

       * equal to last_possible_start, the next memchr call would be

       * "memchr(p, first, 0)", which will return NULL. But it clarifies the

       * logic. */

      return NULL;

    }

  }

  return NULL;

#endif

src/ext/eventdns.c

+3 −2
Original line number Diff line number Diff line
@@ -842,10 +842,11 @@ name_parse(u8 *packet, int length, int *idx, char *name_out, size_t name_out_len 
		}

		if (label_len > 63) return -1;

		if (cp != name_out) {

			if (cp + 1 >= end) return -1;

			if (cp >= name_out + name_out_len - 1) return -1;

			*cp++ = '.';

		}

		if (cp + label_len >= end) return -1;

		if (label_len > name_out_len ||

			cp >= name_out + name_out_len - label_len) return -1;

		memcpy(cp, packet + j, label_len);

		cp += label_len;

		j += label_len;

src/or/channeltls.c

+15 −8
Original line number Diff line number Diff line
@@ -1282,7 +1282,6 @@ static void 
channel_tls_process_versions_cell(var_cell_t *cell, channel_tls_t *chan)

{

  int highest_supported_version = 0;

  const uint8_t *cp, *end;

  int started_here = 0;



  tor_assert(cell);
@@ -1322,12 +1321,16 @@ channel_tls_process_versions_cell(var_cell_t *cell, channel_tls_t *chan) 
  }



  tor_assert(chan->conn->handshake_state);

  end = cell->payload + cell->payload_len;

  for (cp = cell->payload; cp+1 < end; cp += 2) {



  {

    int i;

    const uint8_t *cp = cell->payload;

    for (i = 0; i < cell->payload_len / 2; ++i, cp += 2) {

      uint16_t v = ntohs(get_uint16(cp));

      if (is_or_protocol_version_known(v) && v > highest_supported_version)

        highest_supported_version = v;

    }

  }

  if (!highest_supported_version) {

    log_fn(LOG_PROTOCOL_WARN, LD_OR,

           "Couldn't find a version in common between my version list and the "
@@ -1685,12 +1688,16 @@ channel_tls_process_certs_cell(var_cell_t *cell, channel_tls_t *chan) 
  for (i = 0; i < n_certs; ++i) {

    uint8_t cert_type;

    uint16_t cert_len;

    if (ptr + 3 > cell->payload + cell->payload_len) {

    if (cell->payload_len < 3)

      goto truncated;

    if (ptr > cell->payload + cell->payload_len - 3) {

      goto truncated;

    }

    cert_type = *ptr;

    cert_len = ntohs(get_uint16(ptr+1));

    if (ptr + 3 + cert_len > cell->payload + cell->payload_len) {

    if (cell->payload_len < 3 + cert_len)

      goto truncated;

    if (ptr > cell->payload + cell->payload_len - cert_len - 3) {

      goto truncated;

    }

    if (cert_type == OR_CERT_TYPE_TLS_LINK ||

src/test/test_util.c

+4 −0
Original line number Diff line number Diff line
@@ -1212,6 +1212,10 @@ test_util_strmisc(void) 
    test_assert(!tor_memmem(haystack, 4, "cde", 3));

    haystack = "ababcad";

    test_eq_ptr(tor_memmem(haystack, 7, "abc", 3), haystack + 2);

    test_eq_ptr(tor_memmem(haystack, 7, "ad", 2), haystack + 5);

    test_eq_ptr(tor_memmem(haystack, 7, "cad", 3), haystack + 4);

    test_assert(!tor_memmem(haystack, 7, "dadad", 5));

    test_assert(!tor_memmem(haystack, 7, "abcdefghij", 10));

    /* memstr */

    test_eq_ptr(tor_memstr(haystack, 7, "abc"), haystack + 2);

    test_eq_ptr(tor_memstr(haystack, 7, "cad"), haystack + 4);