[tor-commits] [tor/master] Fix a memory leak on decryption non-failure of v3 hsdesc

[nickm at torproject.org]

commit 5240afa713581a0bbba64547e00107a9cbf17f21
Author: Nick Mathewson <nickm at torproject.org>
Date:   Sun Nov 5 12:21:16 2017 -0500

    Fix a memory leak on decryption non-failure of v3 hsdesc
    
    If it decrypts something that turns out to start with a NUL byte,
    then decrypt_desc_layer() will return 0 to indicate the length of
    its result.  But 0 also indicates an error, which causes the result
    not to be freed by decrypt_desc_layer()'s callers.
    
    Since we're trying to stabilize 0.3.2.x, I've opted for the simpler
    possible fix here and made it so that an empty decrypted string will
    also count as an error.
    
    Fixes bug 24150 and OSS-Fuzz issue 3994.
    
    The original bug was present but unreachable in 0.3.1.1-alpha. I'm
    calling this a bugfix on 0.3.2.1-alpha since that's the first version
    where you could actually try to decrypt these descriptors.
---
 changes/bug24150              |  4 ++++
 src/or/hs_descriptor.c        | 11 ++++++++++-
 src/test/fuzz/fuzz_hsdescv3.c |  8 +++++++-
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/changes/bug24150 b/changes/bug24150
new file mode 100644
index 000000000..cfda7c40d
--- /dev/null
+++ b/changes/bug24150
@@ -0,0 +1,4 @@
+  o Minor bugfixes (v3 onion services):
+    - Fix a memory leak when decrypting a badly formatted v3 onion
+      service descriptor. Fixes bug 24150; bugfix on 0.3.2.1-alpha.
+      Found by OSS-Fuzz; this is OSS-Fuzz issue 3994.
diff --git a/src/or/hs_descriptor.c b/src/or/hs_descriptor.c
index a8ff3471c..170886694 100644
--- a/src/or/hs_descriptor.c
+++ b/src/or/hs_descriptor.c
@@ -1302,7 +1302,11 @@ encrypted_data_length_is_valid(size_t len)
  *  <b>encrypted_blob_size</b>. Use the descriptor object <b>desc</b> to
  *  generate the right decryption keys; set <b>decrypted_out</b> to the
  *  plaintext. If <b>is_superencrypted_layer</b> is set, this is the outter
- *  encrypted layer of the descriptor. */
+ *  encrypted layer of the descriptor.
+ *
+ * On any error case, including an empty output, return 0 and set
+ * *<b>decrypted_out</b> to NULL.
+ */
 MOCK_IMPL(STATIC size_t,
 decrypt_desc_layer,(const hs_descriptor_t *desc,
                     const uint8_t *encrypted_blob,
@@ -1382,6 +1386,11 @@ decrypt_desc_layer,(const hs_descriptor_t *desc,
     }
   }
 
+  if (result_len == 0) {
+    /* Treat this as an error, so that somebody will free the output. */
+    goto err;
+  }
+
   /* Make sure to NUL terminate the string. */
   decrypted[encrypted_len] = '\0';
   *decrypted_out = (char *) decrypted;
diff --git a/src/test/fuzz/fuzz_hsdescv3.c b/src/test/fuzz/fuzz_hsdescv3.c
index 30e82c925..428774e33 100644
--- a/src/test/fuzz/fuzz_hsdescv3.c
+++ b/src/test/fuzz/fuzz_hsdescv3.c
@@ -50,7 +50,13 @@ mock_decrypt_desc_layer(const hs_descriptor_t *desc,
   *decrypted_out = tor_memdup_nulterm(
                    encrypted_blob + HS_DESC_ENCRYPTED_SALT_LEN,
                    encrypted_blob_size - overhead);
-  return strlen(*decrypted_out);
+  size_t result = strlen(*decrypted_out);
+  if (result) {
+    return result;
+  } else {
+    tor_free(*decrypted_out);
+    return 0;
+  }
 }
 
 int