commit 5e3c8a61aba7d9183f2cdbe03ed51a79025d8e48
author Adam Langley <agl@google.com> Mon Aug 13 14:25:46 2018 -0700
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Mon Aug 13 21:42:06 2018 +0000
tree 97cb886b5414050d9f5aedae84387fa305c13729
parent d451453067cd665a5c38830fbbaac9e599234a5e

Bound two other cases of PKCS#12 iteration counts.

The fuzzer found another place where it could cause a timeout by
providing a huge PBKDF2 iteration count. This change bounds another two
places where we parse out iteration counts and that's hopefully all of
them.

BUG=oss-fuzz:9853

Change-Id: I037fa09d2bee79e7435a9d40cbd89c07b4a9d443
Reviewed-on: https://boringssl-review.googlesource.com/30944
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

crypto/pkcs8/internal.h

diff --git a/crypto/pkcs8/internal.h b/crypto/pkcs8/internal.h
index 97c86c4..c3302f7 100644
--- a/crypto/pkcs8/internal.h
+++ b/crypto/pkcs8/internal.h
@@ -119,6 +119,10 @@
                             const char *pass, size_t pass_len,
                             const uint8_t *salt, size_t salt_len);
 
+// pkcs12_iterations_acceptable returns one if |iterations| is a reasonable
+// number of PBKDF2 iterations and zero otherwise.
+int pkcs12_iterations_acceptable(uint64_t iterations);
+
 
 #if defined(__cplusplus)
 }  // extern C

crypto/pkcs8/p5_pbev2.c

diff --git a/crypto/pkcs8/p5_pbev2.c b/crypto/pkcs8/p5_pbev2.c
index 6686cf3..7497b00 100644
--- a/crypto/pkcs8/p5_pbev2.c
+++ b/crypto/pkcs8/p5_pbev2.c
@@ -244,7 +244,7 @@
     return 0;
   }
 
-  if (iterations == 0 || iterations > UINT_MAX) {
+  if (!pkcs12_iterations_acceptable(iterations)) {
     OPENSSL_PUT_ERROR(PKCS8, PKCS8_R_BAD_ITERATION_COUNT);
     return 0;
   }

crypto/pkcs8/pkcs8.c

diff --git a/crypto/pkcs8/pkcs8.c b/crypto/pkcs8/pkcs8.c
index 3453113..ee25ee2 100644
--- a/crypto/pkcs8/pkcs8.c
+++ b/crypto/pkcs8/pkcs8.c
@@ -268,7 +268,7 @@
     return 0;
   }
 
-  if (iterations == 0 || iterations > UINT_MAX) {
+  if (!pkcs12_iterations_acceptable(iterations)) {
     OPENSSL_PUT_ERROR(PKCS8, PKCS8_R_BAD_ITERATION_COUNT);
     return 0;
   }

crypto/pkcs8/pkcs8_x509.c

diff --git a/crypto/pkcs8/pkcs8_x509.c b/crypto/pkcs8/pkcs8_x509.c
index c5a0651..dc74d96 100644
--- a/crypto/pkcs8/pkcs8_x509.c
+++ b/crypto/pkcs8/pkcs8_x509.c
@@ -75,6 +75,21 @@
 #include "../internal.h"
 
 
+int pkcs12_iterations_acceptable(uint64_t iterations) {
+#if defined(BORINGSSL_UNSAFE_FUZZER_MODE)
+  static const uint64_t kIterationsLimit = 2048;
+#else
+  // Windows imposes a limit of 600K. Mozilla say: “so them increasing
+  // maximum to something like 100M or 1G (to have few decades of breathing
+  // room) would be very welcome”[1]. So here we set the limit to 100M.
+  //
+  // [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1436873#c14
+  static const uint64_t kIterationsLimit = 100 * 1000000;
+#endif
+
+  return 0 < iterations && iterations <= kIterationsLimit;
+}
+
 // Minor tweak to operation: zero private key data
 static int pkey_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it,
                    void *exarg) {
@@ -669,22 +684,11 @@
       goto err;
     }
 
-#if defined(BORINGSSL_UNSAFE_FUZZER_MODE)
-    static const uint64_t kIterationsLimit = 2048;
-#else
-    // Windows imposes a limit of 600K. Mozilla say: “so them increasing
-    // maximum to something like 100M or 1G (to have few decades of breathing
-    // room) would be very welcome”[1]. So here we set the limit to 100M.
-    //
-    // [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1436873#c14
-    static const uint64_t kIterationsLimit = 100 * 1000000;
-#endif
-
     // The iteration count is optional and the default is one.
     uint64_t iterations = 1;
     if (CBS_len(&mac_data) > 0) {
       if (!CBS_get_asn1_uint64(&mac_data, &iterations) ||
-          iterations > kIterationsLimit) {
+          !pkcs12_iterations_acceptable(iterations)) {
         OPENSSL_PUT_ERROR(PKCS8, PKCS8_R_BAD_PKCS12_DATA);
         goto err;
       }