Commit 0d4a689d authored by Tobias Stoeckmann's avatar Tobias Stoeckmann Committed by Nick Mathewson
Browse files

Prevent UB on signed overflow. 



Overflowing a signed integer in C is an undefined behaviour.
It is possible to trigger this undefined behaviour in tor_asprintf on
Windows or systems lacking vasprintf.

On these systems, eiter _vscprintf or vsnprintf is called to retrieve
the required amount of bytes to hold the string. These functions can
return INT_MAX. The easiest way to recreate this is the use of a
specially crafted configuration file, e.g. containing the line:

FirewallPorts AAAAA<in total 2147483610 As>

This line triggers the needed tor_asprintf call which eventually
leads to an INT_MAX return value from _vscprintf or vsnprintf.

The needed byte for \0 is added to the result, triggering the
overflow and therefore the undefined behaviour.

Casting the value to size_t before addition fixes the behaviour.

Signed-off-by: default avatarTobias Stoeckmann <tobias@stoeckmann.org>
parent fb977f8c

src/common/compat.c

+4 −5
Original line number Diff line number Diff line
@@ -540,8 +540,8 @@ tor_vasprintf(char **strp, const char *fmt, va_list args) 
    *strp = NULL;

    return -1;

  }

  strp_tmp = tor_malloc(len + 1);

  r = _vsnprintf(strp_tmp, len+1, fmt, args);

  strp_tmp = tor_malloc((size_t)len + 1);

  r = _vsnprintf(strp_tmp, (size_t)len+1, fmt, args);

  if (r != len) {

    tor_free(strp_tmp);

    *strp = NULL;
@@ -566,9 +566,9 @@ tor_vasprintf(char **strp, const char *fmt, va_list args) 
    *strp = tor_strdup(buf);

    return len;

  }

  strp_tmp = tor_malloc(len+1);

  strp_tmp = tor_malloc((size_t)len+1);

  /* use of tor_vsnprintf() will ensure string is null terminated */

  r = tor_vsnprintf(strp_tmp, len+1, fmt, args);

  r = tor_vsnprintf(strp_tmp, (size_t)len+1, fmt, args);

  if (r != len) {

    tor_free(strp_tmp);

    *strp = NULL;
@@ -3543,4 +3543,3 @@ tor_get_avail_disk_space(const char *path) 
  return -1;

#endif

}