commit 0d4a689d3ae8f7e05b3baf8ad71d983a767ef55b Author: Tobias Stoeckmann tobias@stoeckmann.org Date: Mon Jun 24 22:08:49 2019 +0200
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: Tobias Stoeckmann tobias@stoeckmann.org --- src/common/compat.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/common/compat.c b/src/common/compat.c index 975875112..6f7ac7bd7 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -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 } -