Never read off the end of a buffer in base32_encode()

When we "fixed" #18280 in 4e4a7d2b0c
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.
This commit is contained in:
Nick Mathewson 2017-04-07 10:47:16 -04:00
parent 7d7770f735
commit 4812441d34
2 changed files with 9 additions and 3 deletions

5
changes/bug21894_029 Normal file
View File

@ -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.

View File

@ -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];