Reduce the BER conversion recursion depth

2048 is too high, particularly in heavily instrumented fuzzer builds
with large stack frames. Use a much more conservative limit.

Change-Id: If0b49f2ca04520c41400dcbfd83463766fded3a9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65508
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

crypto/bytestring/ber.c

@@ -18,13 +18,10 @@
 #include <string.h>
 
 #include "internal.h"
-#include "../internal.h"
 
 
-// kMaxDepth is a just a sanity limit. The code should be such that the length
-// of the input being processes always decreases. None the less, a very large
-// input could otherwise cause the stack to overflow.
-static const uint32_t kMaxDepth = 2048;
+// kMaxDepth limits the recursion depth to avoid overflowing the stack.
+static const uint32_t kMaxDepth = 128;
 
 // is_string_type returns one if |tag| is a string type and zero otherwise. It
 // ignores the constructed bit.

crypto/bytestring/bytestring_test.cc

@@ -730,6 +730,22 @@ TEST(CBSTest, BerConvert) {
   ExpectBerConvert("kWrappedIndefBER", kWrappedIndefDER, kWrappedIndefBER);
   ExpectBerConvert("kWrappedConstructedStringBER", kWrappedConstructedStringDER,
                    kWrappedConstructedStringBER);
+
+  // indef_overflow is 200 levels deep of an indefinite-length-encoded SEQUENCE.
+  // This will exceed our recursion limits and fail to be converted.
+  std::vector<uint8_t> indef_overflow;
+  for (int i = 0; i < 200; i++) {
+    indef_overflow.push_back(0x30);
+    indef_overflow.push_back(0x80);
+  }
+  for (int i = 0; i < 200; i++) {
+    indef_overflow.push_back(0x00);
+    indef_overflow.push_back(0x00);
+  }
+  CBS in, out;
+  CBS_init(&in, indef_overflow.data(), indef_overflow.size());
+  uint8_t *storage;
+  ASSERT_FALSE(CBS_asn1_ber_to_der(&in, &out, &storage));
 }
 
 struct BERTest {

crypto/pkcs8/pkcs12_test.cc

@@ -158,6 +158,14 @@ TEST(PKCS12Test, TestEmptyPassword) {
            nullptr);
   TestImpl("EmptyPassword (BER, null password)", StringToBytes(data), nullptr,
            nullptr);
+
+  // The constructed string with too much recursion.
+  data = GetTestData("crypto/pkcs8/test/empty_password_ber_nested.p12");
+  bssl::UniquePtr<STACK_OF(X509)> certs(sk_X509_new_null());
+  ASSERT_TRUE(certs);
+  EVP_PKEY *key = nullptr;
+  CBS pkcs12 = StringToBytes(data);
+  EXPECT_FALSE(PKCS12_get_key_and_certs(&key, certs.get(), &pkcs12, ""));
 }
 
 TEST(PKCS12Test, TestNullPassword) {

sources.cmake

@@ -148,6 +148,7 @@ set(
   crypto/kyber/kyber_tests.txt
   crypto/pkcs8/test/empty_password.p12
   crypto/pkcs8/test/empty_password_ber.p12
+  crypto/pkcs8/test/empty_password_ber_nested.p12
   crypto/pkcs8/test/no_encryption.p12
   crypto/pkcs8/test/nss.p12
   crypto/pkcs8/test/null_password.p12