From 0d5a0b4f0ccc804913fbca20acf5fc62f52570b8 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Wed, 29 May 2019 09:33:24 -0400 Subject: Fixed tor_vasprintf on systems without vasprintf. If tor is compiled on a system with neither vasprintf nor _vscprintf, the fallback implementation exposes a logic flaw which prevents proper usage of strings longer than 127 characters: * tor_vsnprintf returns -1 if supplied buffer is not large enough, but tor_vasprintf uses this function to retrieve required length * the result of tor_vsnprintf is not properly checked for negative return values Both aspects together could in theory lead to exposure of uninitialized stack memory in the resulting string. This requires an invalid format string or data that exceeds integer limitations. Fortunately tor is not even able to run with this implementation because it runs into asserts early on during startup. Also the unit tests fail during a "make check" run. Signed-off-by: Tobias Stoeckmann [backported to 0.2.9 by nickm] --- src/common/compat.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/common/compat.c b/src/common/compat.c index 9758751122..d3bc2f5fec 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -558,10 +558,17 @@ tor_vasprintf(char **strp, const char *fmt, va_list args) int len, r; va_list tmp_args; va_copy(tmp_args, args); - /* vsnprintf() was properly checked but tor_vsnprintf() available so - * why not use it? */ - len = tor_vsnprintf(buf, sizeof(buf), fmt, tmp_args); + /* Use vsnprintf to retrieve needed length. tor_vsnprintf() is not an option + * here because it will simply return -1 if buf is not large enough to hold the + * complete string. + */ + len = vsnprintf(buf, sizeof(buf), fmt, tmp_args); va_end(tmp_args); + buf[sizeof(buf) - 1] = '\0'; + if (len < 0) { + *strp = NULL; + return -1; + } if (len < (int)sizeof(buf)) { *strp = tor_strdup(buf); return len; -- cgit v1.2.3-54-g00ecf From 0e0cf4abd80249faa23f2bbdb89e62ba96c898f0 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 29 May 2019 09:38:32 -0400 Subject: Tweak comments in tor_vasprintf(), and add a changes file for 30651 --- changes/bug30561 | 6 ++++++ src/common/compat.c | 10 ++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) create mode 100644 changes/bug30561 diff --git a/changes/bug30561 b/changes/bug30561 new file mode 100644 index 0000000000..afb3f02c62 --- /dev/null +++ b/changes/bug30561 @@ -0,0 +1,6 @@ + o Minor bugfixes (portability): + - Avoid crashing in our tor_vasprintf() implementation on systems that + define neither vasprintf() nor _vscprintf(). (This bug has been here + long enough that we question whether people are running Tor on such + systems, but we're applying the fix out of caution.) Fixes bug 30561; + bugfix on 0.2.8.2-alpha. Found and fixed by Tobias Stoeckmann. diff --git a/src/common/compat.c b/src/common/compat.c index d3bc2f5fec..ee3bf0fd50 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -554,13 +554,16 @@ tor_vasprintf(char **strp, const char *fmt, va_list args) * characters we need. We give it a try on a short buffer first, since * it might be nice to avoid the second vsnprintf call. */ + /* XXXX This code spent a number of years broken (see bug 30651). It is + * possible that no Tor users actually run on systems without vasprintf() or + * _vscprintf(). If so, we should consider removing this code. */ char buf[128]; int len, r; va_list tmp_args; va_copy(tmp_args, args); - /* Use vsnprintf to retrieve needed length. tor_vsnprintf() is not an option - * here because it will simply return -1 if buf is not large enough to hold the - * complete string. + /* Use vsnprintf to retrieve needed length. tor_vsnprintf() is not an + * option here because it will simply return -1 if buf is not large enough + * to hold the complete string. */ len = vsnprintf(buf, sizeof(buf), fmt, tmp_args); va_end(tmp_args); @@ -3550,4 +3553,3 @@ tor_get_avail_disk_space(const char *path) return -1; #endif } - -- cgit v1.2.3-54-g00ecf