commit 17458a87d73f605fe6a4bb3c23cf2de135eec99c 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/lib/string/printf.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/lib/string/printf.c b/src/lib/string/printf.c index 415d4ac4a..00659337e 100644 --- a/src/lib/string/printf.c +++ b/src/lib/string/printf.c @@ -117,8 +117,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; @@ -143,9 +143,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;