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>
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
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; }
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; }
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; }