[tor-commits] [tor/master] Never read off the end of a buffer in base32_encode()

[nickm at torproject.org]

commit 4812441d3465f4f2fc6763ee644f79d5a9c8661b
Author: Nick Mathewson <nickm at torproject.org>
Date:   Fri Apr 7 10:47:16 2017 -0400

    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(-)

diff --git a/changes/bug21894_029 b/changes/bug21894_029
new file mode 100644
index 0000000..e3a84fa
--- /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 aef9db8..a460842 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<nbits)
-      v += (uint8_t)src[(bit/8)+1];
+    size_t idx = bit / 8;
+    v = ((uint8_t)src[idx]) << 8;
+    if (idx+1 < srclen)
+      v += (uint8_t)src[idx+1];
     /* set u to the 5-bit value at the bit'th bit of buf. */
     u = (v >> (11-(bit%8))) & 0x1F;
     dest[i] = BASE32_CHARS[u];