From 4812441d3465f4f2fc6763ee644f79d5a9c8661b Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 7 Apr 2017 10:47:16 -0400 Subject: Never read off the end of a buffer in base32_encode() When we "fixed" #18280 in 4e4a7d2b0c199227252a742541461ec4cc35d358 in 0291 it appears that we introduced a bug: The base32_encode function can read off the end of the input buffer, if the input buffer size modulo 5 is not equal to 0 or 3. This is not completely horrible, for two reasons: * The extra bits that are read are never actually used: so this is only a crash when asan is enabled, in the worst case. Not a data leak. * The input sizes passed to base32_encode are only ever multiples of 5. They are all either DIGEST_LEN (20), REND_SERVICE_ID_LEN (10), sizeof(rand_bytes) in addressmap.c (10), or an input in crypto.c that is forced to a multiple of 5. So this bug can't actually trigger in today's Tor. Closes bug 21894; bugfix on 0.2.9.1-alpha. --- changes/bug21894_029 | 5 +++++ src/common/util_format.c | 7 ++++--- 2 files changed, 9 insertions(+), 3 deletions(-) create mode 100644 changes/bug21894_029 diff --git a/changes/bug21894_029 b/changes/bug21894_029 new file mode 100644 index 0000000000..e3a84fa721 --- /dev/null +++ b/changes/bug21894_029 @@ -0,0 +1,5 @@ + o Minor bugfixes (crash prevention): + - Fix an (currently untriggerable, but potentially dangerous) crash + bug when base32-encoding inputs whose sizes are not a multiple of + 5. Fixes bug 21894; bugfix on 0.2.9.1-alpha. + diff --git a/src/common/util_format.c b/src/common/util_format.c index aef9db85c8..a46084223a 100644 --- a/src/common/util_format.c +++ b/src/common/util_format.c @@ -51,9 +51,10 @@ base32_encode(char *dest, size_t destlen, const char *src, size_t srclen) for (i=0,bit=0; bit < nbits; ++i, bit+=5) { /* set v to the 16-bit value starting at src[bits/8], 0-padded. */ - v = ((uint8_t)src[bit/8]) << 8; - if (bit+5> (11-(bit%8))) & 0x1F; dest[i] = BASE32_CHARS[u]; -- cgit v1.2.3-54-g00ecf