From 0d4a689d3ae8f7e05b3baf8ad71d983a767ef55b Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Mon, 24 Jun 2019 22:08:49 +0200 Subject: [PATCH 1/4] 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 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 --- 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 9758751122..6f7ac7bd7d 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 } - From 97d73db7c36ec3fac2974726012f76bff63f9dfc Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 19 Jul 2019 09:21:08 -0400 Subject: [PATCH 2/4] Changes file for bug 31001 --- changes/ticket31001 | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changes/ticket31001 diff --git a/changes/ticket31001 b/changes/ticket31001 new file mode 100644 index 0000000000..2ce1cbdf34 --- /dev/null +++ b/changes/ticket31001 @@ -0,0 +1,6 @@ + o Minor bugfixes (compatibility, standards compliance): + - Fix a bug that would invoke undefined behavior on certain operating + systems when trying to asprintf() a string exactly INT_MAX bytes + long. We don't believe this is exploitable, but it's better + to fix it anyway. Fixes bug 31001; bugfix on 0.2.2.11-alpha. + Found and fixed by Tobias Stoeckmann. From 6ed3d8dbd180aa72e2c9b3fa4835fb82e0f60556 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 23 Sep 2019 15:04:54 -0400 Subject: [PATCH 3/4] Try using make -k in Travis configuration Frequently, when a patch fails, it has failures in several files. Using the "-k" flag will let us learn all the compilation errors, not just the first one that the compiler hits. Based on a patch by rl1987. --- .travis.yml | 6 ++++-- changes/ticket31372_travis | 4 ++++ 2 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 changes/ticket31372_travis diff --git a/.travis.yml b/.travis.yml index a2bc6395df..7597107372 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,8 +15,10 @@ os: ## OS and compiler. env: global: - ## The Travis CI environment allows us two cores, so let's use both. - - MAKEFLAGS="-j 2" + ## The Travis CI environment allows us two cores, so let's use both. Also, + ## let's use the "-k" flag so that we get all of the compilation failures, + ## not just the first one. + - MAKEFLAGS="-k -j 2" ## We turn on hardening by default ## Also known as --enable-fragile-hardening in 0.3.0.3-alpha and later - HARDENING_OPTIONS="--enable-expensive-hardening" diff --git a/changes/ticket31372_travis b/changes/ticket31372_travis new file mode 100644 index 0000000000..403869b2ed --- /dev/null +++ b/changes/ticket31372_travis @@ -0,0 +1,4 @@ + o Minor features (continuous integration): + - When building on Travis, pass the "-k" flag to make, so that + we are informed of all compilation failures, not just the first + one or two. Closes part of ticket 31372. From 49a11f8a8a61a4d8b8657a98291862ffc96e99d9 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 23 Sep 2019 15:04:54 -0400 Subject: [PATCH 4/4] Try using make -k in the Appveyor configuration Frequently, when a patch fails, it has failures in several files. Using the "-k" flag will let us learn all the compilation errors, not just the first one that the compiler hits. Based on a patch by rl1987. Closes ticket 31372. --- .appveyor.yml | 6 +++--- changes/ticket31372_appveyor | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) create mode 100644 changes/ticket31372_appveyor diff --git a/.appveyor.yml b/.appveyor.yml index 9913bf3bd9..25f87fa457 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -83,8 +83,8 @@ build_script: # configure its flags. liblzma just works. #> Execute-Bash "ZSTD_CFLAGS='-L/${env:compiler_path}/include' ZSTD_LIBS='-L/${env:compiler_path}/lib -lzstd' ../configure --prefix=/${env:compiler_path} --build=${env:target} --host=${env:target} --with-openssl-dir=/${env:compiler_path} --disable-asciidoc --enable-fatal-warnings ${env:hardening}" - Execute-Bash "V=1 make -j2" - Execute-Bash "V=1 make -j2 install" + Execute-Bash "V=1 make -k -j2" + Execute-Bash "V=1 make -k -j2 install" } test_script: @@ -95,7 +95,7 @@ test_script: $buildpath = @("C:\msys64\${env:compiler_path}\bin") + $oldpath $env:Path = $buildpath -join ';' Set-Location "${env:build}" - Execute-Bash "VERBOSE=1 make -j2 check" + Execute-Bash "VERBOSE=1 make -k -j2 check" } on_finish: diff --git a/changes/ticket31372_appveyor b/changes/ticket31372_appveyor new file mode 100644 index 0000000000..e7bb03182e --- /dev/null +++ b/changes/ticket31372_appveyor @@ -0,0 +1,4 @@ + o Minor features (continuous integration): + - When building on Appveyor, pass the "-k" flag to make, so that + we are informed of all compilation failures, not just the first + one or two. Closes part of ticket 31372.