Avoid uncontrolled recusion on indefinite length encodings

A sufficiently nested indefinite length construction would cause
stack exhaustion and a crash. Found by OSS-Fuzz - issue 4353

2 files changed, 69 insertions, 17 deletions

diff --git a/src/lib/asn1/ber_dec.cpp b/src/lib/asn1/ber_dec.cpp
index dd83e2446..515f7352f 100644
--- a/src/lib/asn1/ber_dec.cpp
+++ b/src/lib/asn1/ber_dec.cpp
@@ -1,7 +1,6 @@
-
 /*
 * BER Decoder
-* (C) 1999-2008,2015 Jack Lloyd
+* (C) 1999-2008,2015,2017 Jack Lloyd
 *
 * Botan is released under the Simplified BSD License (see license.txt)
 */
@@ -16,6 +15,13 @@ namespace Botan {
 namespace {
 
 /*
+* This value is somewhat arbitrary. OpenSSL allows up to 128 nested
+* indefinite length sequences. If you increase this, also increase the
+* limit in the test in test_asn1.cpp
+*/
+const size_t ALLOWED_EOC_NESTINGS = 16;
+
+/*
 * BER decode an ASN.1 type tag
 */
 size_t decode_tag(DataSource* ber, ASN1_Tag& type_tag, ASN1_Tag& class_tag)
@@ -55,12 +61,12 @@ size_t decode_tag(DataSource* ber, ASN1_Tag& type_tag, ASN1_Tag& class_tag)
 /*
 * Find the EOC marker
 */
-size_t find_eoc(DataSource*);
+size_t find_eoc(DataSource* src, size_t allow_indef);
 
 /*
 * BER decode an ASN.1 length field
 */
-size_t decode_length(DataSource* ber, size_t& field_size)
+size_t decode_length(DataSource* ber, size_t& field_size, size_t allow_indef)
    {
    uint8_t b;
    if(!ber->read_byte(b))
@@ -70,10 +76,21 @@ size_t decode_length(DataSource* ber, size_t& field_size)
       return b;
 
    field_size += (b & 0x7F);
-   if(field_size == 1) return find_eoc(ber);
    if(field_size > 5)
       throw BER_Decoding_Error("Length field is too large");
 
+   if(field_size == 1)
+      {
+      if(allow_indef == 0)
+         {
+         throw BER_Decoding_Error("Nested EOC markers too deep, rejecting to avoid stack exhaustion");
+         }
+      else
+         {
+         return find_eoc(ber, allow_indef - 1);
+         }
+      }
+
    size_t length = 0;
 
    for(size_t i = 0; i != field_size - 1; ++i)
@@ -88,18 +105,9 @@ size_t decode_length(DataSource* ber, size_t& field_size)
    }
 
 /*
-* BER decode an ASN.1 length field
-*/
-size_t decode_length(DataSource* ber)
-   {
-   size_t dummy;
-   return decode_length(ber, dummy);
-   }
-
-/*
 * Find the EOC marker
 */
-size_t find_eoc(DataSource* ber)
+size_t find_eoc(DataSource* ber, size_t allow_indef)
    {
    secure_vector<uint8_t> buffer(DEFAULT_BUFFERSIZE), data;
 
@@ -124,7 +132,7 @@ size_t find_eoc(DataSource* ber)
          break;
 
       size_t length_size = 0;
-      size_t item_size = decode_length(&source, length_size);
+      size_t item_size = decode_length(&source, length_size, allow_indef);
       source.discard_next(item_size);
 
       length = BOTAN_CHECKED_ADD(length, item_size);
@@ -224,7 +232,8 @@ BER_Object BER_Decoder::get_next_object()
       if(next.type_tag == NO_OBJECT)
          return next;
 
-      const size_t length = decode_length(m_source);
+      size_t field_size;
+      const size_t length = decode_length(m_source, field_size, ALLOWED_EOC_NESTINGS);
       if(!m_source->check_available(length))
          throw BER_Decoding_Error("Value truncated");
 
diff --git a/src/tests/test_asn1.cpp b/src/tests/test_asn1.cpp
index c99fa41d9..633f57602 100644
--- a/src/tests/test_asn1.cpp
+++ b/src/tests/test_asn1.cpp
@@ -10,6 +10,7 @@
    #include <botan/der_enc.h>
    #include <botan/ber_dec.h>
    #include <botan/asn1_str.h>
+   #include <botan/asn1_print.h>
 #endif
 
 namespace Botan_Tests {
@@ -45,6 +46,47 @@ Test::Result test_ber_stack_recursion()
    return result;
    }
 
+Test::Result test_ber_eoc_decoding_limits()
+   {
+   Test::Result result("BER nested indefinite length");
+
+   // OSS-Fuzz #4353
+
+   Botan::ASN1_Pretty_Printer printer;
+
+   size_t max_eoc_allowed = 0;
+
+   for(size_t len = 1; len < 1024; ++len)
+      {
+      std::vector<uint8_t> buf(4*len);
+
+      /*
+      This constructs a len deep sequence of SEQUENCES each with
+      an indefinite length
+      */
+      for(size_t i = 0; i != 2*len; i += 2)
+         {
+         buf[i  ] = 0x30;
+         buf[i+1] = 0x80;
+         }
+      // remainder of values left as zeros (EOC markers)
+
+      try
+         {
+         printer.print(buf);
+         }
+      catch(Botan::BER_Decoding_Error&)
+         {
+         max_eoc_allowed = len - 1;
+         break;
+         }
+      }
+
+   result.test_eq("EOC limited to prevent stack exhaustion", max_eoc_allowed, 16);
+
+   return result;
+   }
+
 Test::Result test_asn1_utf8_ascii_parsing()
    {
    Test::Result result("ASN.1 ASCII parsing");
@@ -234,6 +276,7 @@ class ASN1_Tests final : public Test
          std::vector<Test::Result> results;
 
          results.push_back(test_ber_stack_recursion());
+         results.push_back(test_ber_eoc_decoding_limits());
          results.push_back(test_asn1_utf8_ascii_parsing());
          results.push_back(test_asn1_utf8_parsing());
          results.push_back(test_asn1_ucs2_parsing());