Merge pull request #5710 from randombit/jack/asn1-hardening

ASN.1 hardenings and improved decoder strictness

Author: randombit

src/lib/asn1/asn1_print.cpp

@@ -21,28 +21,10 @@ namespace Botan {
 
 namespace {
 
-// Printable here means fits into an ASN.1 "PRINTABLE STRING" type
-bool is_printable_char(char c) {
-   if(c >= 'a' && c <= 'z') {
-      return true;
-   }
-
-   if(c >= 'A' && c <= 'Z') {
-      return true;
-   }
-
-   if(c >= '0' && c <= '9') {
-      return true;
-   }
-
-   if(c == '.' || c == ':' || c == '/' || c == '-') {
-      return true;
-   }
-
-   return false;
-}
-
 bool all_printable_chars(const uint8_t bits[], size_t bits_len) {
+   // Printable here means fits into an ASN.1 "PRINTABLE STRING" type
+   constexpr auto is_printable_char = CharacterValidityTable::alpha_numeric_plus(".:/-");
+
    for(size_t i = 0; i != bits_len; ++i) {
       if(!is_printable_char(bits[i])) {
          return false;
@@ -83,7 +65,10 @@ std::string ASN1_Formatter::print(const uint8_t in[], size_t len) const {
 }
 
 void ASN1_Formatter::print_to_stream(std::ostream& output, const uint8_t in[], size_t len) const {
-   const auto decoder_limits = m_require_der ? BER_Decoder::Limits::DER() : BER_Decoder::Limits::BER();
+   // The pretty printer is a best-effort diagnostic tool, so in BER mode it
+   // tolerates standalone EOC markers emitted by some BER producers.
+   const auto decoder_limits =
+      m_require_der ? BER_Decoder::Limits::DER() : BER_Decoder::Limits::BER().with_standalone_eoc_allowed();
    BER_Decoder dec(std::span<const uint8_t>{in, len}, decoder_limits);
    decode(output, dec, 0);
 }
@@ -98,29 +83,37 @@ void ASN1_Formatter::decode(std::ostream& output, BER_Decoder& decoder, size_t l
       const ASN1_Class class_tag = obj.get_class();
       const size_t length = obj.length();
 
-      /* hack to insert the tag+length back in front of the stuff now
-         that we've gotten the type info */
-      std::vector<uint8_t> bits;
-      DER_Encoder(bits).add_object(type_tag, class_tag, obj.bits(), obj.length());
-
-      BER_Decoder data(bits, decoder.limits());
-
       if(intersects(class_tag, ASN1_Class::Constructed)) {
-         BER_Decoder cons_info(obj, decoder.limits());
-
          if(recurse_deeper) {
             output << format(type_tag, class_tag, level, length, "");
+            // Move (not copy) the content into the sub-decoder; copying at every
+            // nesting level lets deeply nested input exhaust memory.
+            BER_Decoder cons_info(std::move(obj), decoder.limits());
             decode(output, cons_info, level + 1);  // recurse
          } else {
+            std::vector<uint8_t> bits;
+            DER_Encoder(bits).add_object(type_tag, class_tag, obj.bits(), obj.length());
             output << format(type_tag, class_tag, level, length, format_bin(type_tag, class_tag, bits));
          }
-      } else if(intersects(class_tag, ASN1_Class::Application) || intersects(class_tag, ASN1_Class::ContextSpecific)) {
+
+         obj = decoder.get_next_object();
+         continue;
+      }
+
+      /* hack to insert the tag+length back in front of the stuff now
+         that we've gotten the type info */
+      std::vector<uint8_t> bits;
+      DER_Encoder(bits).add_object(type_tag, class_tag, obj.bits(), obj.length());
+
+      BER_Decoder data(bits, decoder.limits());
+
+      if(intersects(class_tag, ASN1_Class::Application) || intersects(class_tag, ASN1_Class::ContextSpecific)) {
          bool success_parsing_cs = false;
 
          if(m_print_context_specific) {
             try {
                if(possibly_a_general_name(bits.data(), bits.size())) {
-                  output << format(type_tag, class_tag, level, level, bytes_to_string(std::span{bits}.subspan(2)));
+                  output << format(type_tag, class_tag, level, length, bytes_to_string(std::span{bits}.subspan(2)));
                   success_parsing_cs = true;
                } else if(recurse_deeper) {
                   std::vector<uint8_t> inner_bits;
@@ -269,6 +262,13 @@ std::string ASN1_Pretty_Printer::format(
 std::string ASN1_Pretty_Printer::format_bin(ASN1_Type /*type_tag*/,
                                             ASN1_Class /*class_tag*/,
                                             const std::vector<uint8_t>& vec) const {
+   // A value larger than the binary print limit is suppressed by format(), so
+   // skip the (potentially large) string/hex conversion entirely. vec.size() is
+   // a lower bound on the formatted length, so such a value is certainly dropped.
+   if(vec.size() > m_print_binary_limit) {
+      return "";
+   }
+
    if(all_printable_chars(vec.data(), vec.size())) {
       return bytes_to_string(vec);
    } else {

src/lib/asn1/ber_dec.cpp

@@ -12,6 +12,7 @@
 #include <botan/internal/asn1_utils.h>
 #include <botan/internal/int_utils.h>
 #include <botan/internal/loadstor.h>
+#include <algorithm>
 #include <memory>
 
 namespace Botan {
@@ -37,6 +38,12 @@ size_t decode_tag(DataSource* ber, ASN1_Type& type_tag, ASN1_Class& class_tag) {
    if((*b & 0x1F) != 0x1F) {
       type_tag = ASN1_Type(*b & 0x1F);
       class_tag = ASN1_Class(*b & 0xE0);
+      // The EOC marker is primitive; a constructed universal tag 0 has no
+      // valid meaning and would otherwise bypass the EOC handling, which
+      // matches on (Eoc, Universal) exactly
+      if(type_tag == ASN1_Type::Eoc && class_tag == ASN1_Class::Constructed) {
+         throw BER_Decoding_Error("EOC tag with constructed encoding");
+      }
       return 1;
    }
 
@@ -49,7 +56,8 @@ size_t decode_tag(DataSource* ber, ASN1_Type& type_tag, ASN1_Class& class_tag) {
       if(!b) {
          throw BER_Decoding_Error("Long-form tag truncated");
       }
-      if((tag_buf >> 24) != 0) {
+      // Reject if shifting in another 7 bits would overflow the uint32_t tag
+      if((tag_buf >> 25) != 0) {
          throw BER_Decoding_Error("Long-form tag overflowed 32 bits");
       }
       // This is required even by BER (see X.690 section 8.1.2.4.2 sentence c).
@@ -197,6 +205,12 @@ size_t peek_tag(DataSource* src, size_t offset, ASN1_Type& type_tag, ASN1_Class&
    if((b & 0x1F) != 0x1F) {
       type_tag = ASN1_Type(b & 0x1F);
       class_tag = ASN1_Class(b & 0xE0);
+      // The EOC marker is primitive; a constructed universal tag 0 has no
+      // valid meaning and would otherwise bypass the EOC handling, which
+      // matches on (Eoc, Universal) exactly
+      if(type_tag == ASN1_Type::Eoc && class_tag == ASN1_Class::Constructed) {
+         throw BER_Decoding_Error("EOC tag with constructed encoding");
+      }
       return 1;
    }
 
@@ -208,7 +222,8 @@ size_t peek_tag(DataSource* src, size_t offset, ASN1_Type& type_tag, ASN1_Class&
       if(src->peek(&b, 1, offset + tag_bytes) == 0) {
          throw BER_Decoding_Error("Long-form tag truncated");
       }
-      if((tag_buf >> 24) != 0) {
+      // Reject if shifting in another 7 bits would overflow the uint32_t tag
+      if((tag_buf >> 25) != 0) {
          throw BER_Decoding_Error("Long-form tag overflowed 32 bits");
       }
       // Required even by BER (X.690 section 8.1.2.4.2 sentence c).
@@ -246,7 +261,8 @@ size_t peek_tag(DataSource* src, size_t offset, ASN1_Type& type_tag, ASN1_Class&
 * Returns the decoded length and sets field_size to the number of bytes consumed.
 * For indefinite-length encoding, recursively scans ahead to find the EOC marker.
 */
-size_t peek_length(DataSource* src, size_t offset, size_t& field_size, size_t allow_indef, bool constructed) {
+size_t peek_length(
+   DataSource* src, size_t offset, size_t& field_size, size_t allow_indef, bool constructed, bool der_mode) {
    uint8_t b = 0;
    if(src->peek(&b, 1, offset) == 0) {
       throw BER_Decoding_Error("Length field not found");
@@ -264,6 +280,10 @@ size_t peek_length(DataSource* src, size_t offset, size_t& field_size, size_t al
    }
 
    if(num_length_bytes == 0) {
+      // Indefinite length is not allowed in DER
+      if(der_mode) {
+         throw BER_Decoding_Error("Detected indefinite-length encoding in DER structure");
+      }
       // Indefinite length is only valid for constructed types (X.690 8.1.3.2)
       if(!constructed) {
          throw BER_Decoding_Error("Indefinite-length encoding used with non-constructed type");
@@ -303,7 +323,8 @@ size_t find_eoc(DataSource* src, size_t base_offset, size_t allow_indef) {
       }
 
       size_t length_size = 0;
-      const size_t item_size = peek_length(src, offset + tag_size, length_size, allow_indef, is_constructed(class_tag));
+      const size_t item_size =
+         peek_length(src, offset + tag_size, length_size, allow_indef, is_constructed(class_tag), false);
 
       if(auto new_offset = checked_add(offset, tag_size, length_size, item_size)) {
          offset = new_offset.value();
@@ -397,6 +418,39 @@ class DataSource_Span final : public DataSource {
       size_t m_offset = 0;
 };
 
+/*
+* Verify that the elements of a SET appear in sorted order, comparing the full
+* encoding of each element as an octet string. DER requires this canonical
+* ordering. Throws if the elements are not sorted.
+*/
+void verify_set_is_sorted(std::span<const uint8_t> content) {
+   DataSource_Span src(content);
+   size_t offset = 0;
+   std::optional<std::span<const uint8_t>> prev;
+
+   while(offset < content.size()) {
+      ASN1_Type type_tag = ASN1_Type::NoObject;
+      ASN1_Class class_tag = ASN1_Class::NoObject;
+      const size_t tag_size = peek_tag(&src, offset, type_tag, class_tag);
+
+      size_t length_size = 0;
+      const size_t item_size =
+         peek_length(&src, offset + tag_size, length_size, /*allow_indef=*/0, is_constructed(class_tag), true);
+
+      const auto end = checked_add(offset, tag_size, length_size, item_size);
+      if(!end || *end > content.size()) {
+         throw BER_Decoding_Error("SET element exceeds available data");
+      }
+
+      const auto elem = content.subspan(offset, *end - offset);
+      if(prev && std::lexicographical_compare(elem.begin(), elem.end(), prev->begin(), prev->end())) {
+         throw BER_Decoding_Error("Detected unsorted SET in DER structure");
+      }
+      prev = elem;
+      offset = *end;
+   }
+}
+
 }  // namespace
 
 BER_Decoder::~BER_Decoder() = default;
@@ -488,6 +542,10 @@ BER_Object BER_Decoder::get_next_object() {
          throw BER_Decoding_Error("EOC marker with non-zero length");
       }
 
+      if(const auto max_size = m_limits.max_object_size(); max_size && dl.content_length() > *max_size) {
+         throw BER_Decoding_Error("Encoded object exceeds maximum size");
+      }
+
       if(!m_source->check_available(dl.total_length())) {
          throw BER_Decoding_Error("Value truncated");
       }
@@ -509,10 +567,16 @@ BER_Object BER_Decoder::get_next_object() {
          if(m_limits.require_der_encoding()) {
             throw BER_Decoding_Error("Detected EOC marker in DER structure");
          }
-         continue;
-      } else {
-         break;
+         // An EOC marker is only valid as an indefinite-length terminator, which
+         // is consumed above when reading the indefinite-length object. A
+         // standalone EOC is rejected unless the caller opted to tolerate it.
+         if(m_limits.allow_standalone_eoc()) {
+            continue;
+         }
+         throw BER_Decoding_Error("Encountered EOC marker outside of indefinite-length encoding");
       }
+
+      break;
    }
 
    return next;
@@ -550,6 +614,12 @@ void BER_Decoder::push_back(BER_Object&& obj) {
 BER_Decoder BER_Decoder::start_cons(ASN1_Type type_tag, ASN1_Class class_tag) {
    BER_Object obj = get_next_object();
    obj.assert_is_a(type_tag, class_tag | ASN1_Class::Constructed);
+
+   // In DER mode the elements of a universal SET must appear in sorted order
+   if(m_limits.require_der_encoding() && type_tag == ASN1_Type::Set && class_tag == ASN1_Class::Universal) {
+      verify_set_is_sorted(std::span<const uint8_t>{obj.bits(), obj.length()});
+   }
+
    BER_Decoder child(std::move(obj), this);
    return child;
 }
@@ -573,6 +643,11 @@ BER_Decoder::BER_Decoder(BER_Object&& obj, BER_Decoder* parent) :
    m_source = m_data_src.get();
 }
 
+BER_Decoder::BER_Decoder(BER_Object&& obj, Limits limits) : m_limits(limits) {
+   m_data_src = std::make_unique<DataSource_BERObject>(std::move(obj));
+   m_source = m_data_src.get();
+}
+
 /*
 * BER_Decoder Constructor
 */
@@ -697,11 +772,13 @@ BER_Decoder& BER_Decoder::decode(BigInt& out, ASN1_Type type_tag, ASN1_Class cla
    const BER_Object obj = get_next_object();
    obj.assert_is_a(type_tag, class_tag);
 
+   // An INTEGER must have at least one content octet (X.690 section 8.3.1)
+   if(obj.length() == 0) {
+      throw BER_Decoding_Error("INTEGER encoding has no content octets");
+   }
+
    // DER requires minimal INTEGER encoding (X.690 section 8.3.2)
    if(m_limits.require_der_encoding()) {
-      if(obj.length() == 0) {
-         throw BER_Decoding_Error("Detected empty INTEGER encoding in DER structure");
-      }
       if(obj.length() > 1) {
          if(obj.bits()[0] == 0x00 && (obj.bits()[1] & 0x80) == 0) {
             throw BER_Decoding_Error("Detected non-minimal INTEGER encoding in DER structure");
@@ -712,29 +789,25 @@ BER_Decoder& BER_Decoder::decode(BigInt& out, ASN1_Type type_tag, ASN1_Class cla
       }
    }
 
-   if(obj.length() == 0) {
-      out.clear();
-   } else {
-      const uint8_t first = obj.bits()[0];
-      const bool negative = (first & 0x80) == 0x80;
-
-      if(negative) {
-         secure_vector<uint8_t> vec(obj.bits(), obj.bits() + obj.length());
-         for(size_t i = obj.length(); i > 0; --i) {
-            const bool gt0 = (vec[i - 1] > 0);
-            vec[i - 1] -= 1;
-            if(gt0) {
-               break;
-            }
-         }
-         for(size_t i = 0; i != obj.length(); ++i) {
-            vec[i] = ~vec[i];
+   const uint8_t first = obj.bits()[0];
+   const bool negative = (first & 0x80) == 0x80;
+
+   if(negative) {
+      secure_vector<uint8_t> vec(obj.bits(), obj.bits() + obj.length());
+      for(size_t i = obj.length(); i > 0; --i) {
+         const bool gt0 = (vec[i - 1] > 0);
+         vec[i - 1] -= 1;
+         if(gt0) {
+            break;
          }
-         out._assign_from_bytes(vec);
-         out.flip_sign();
-      } else {
-         out._assign_from_bytes(obj.data());
       }
+      for(size_t i = 0; i != obj.length(); ++i) {
+         vec[i] = ~vec[i];
+      }
+      out._assign_from_bytes(vec);
+      out.flip_sign();
+   } else {
+      out._assign_from_bytes(obj.data());
    }
 
    return (*this);
@@ -755,9 +828,10 @@ void asn1_decode_binary_string(std::vector<uint8_t, Alloc>& buffer,
                                bool require_der) {
    obj.assert_is_a(type_tag, class_tag);
 
-   // DER requires BIT STRING and OCTET STRING to use primitive encoding
-   if(require_der && is_constructed(obj)) {
-      throw BER_Decoding_Error("Detected constructed string encoding in DER structure");
+   // Only primitive OCTET STRING/BIT STRING are supported; constructed
+   // (fragmented) BER forms are rejected rather than concatenated.
+   if(is_constructed(obj)) {
+      throw BER_Decoding_Error("Constructed OCTET STRING/BIT STRING is not supported");
    }
 
    if(real_type == ASN1_Type::OctetString) {
@@ -797,8 +871,10 @@ void asn1_decode_binary_string(std::vector<uint8_t, Alloc>& buffer,
 uint8_t asn1_bitstring_unused_bits(const BER_Object& obj, ASN1_Type type_tag, ASN1_Class class_tag, bool require_der) {
    obj.assert_is_a(type_tag, class_tag);
 
-   if(require_der && is_constructed(obj)) {
-      throw BER_Decoding_Error("Detected constructed string encoding in DER structure");
+   // Only primitive BIT STRING is supported; constructed (fragmented) BER
+   // forms are rejected rather than concatenated.
+   if(is_constructed(obj)) {
+      throw BER_Decoding_Error("Constructed OCTET STRING/BIT STRING is not supported");
    }
 
    if(obj.length() == 0) {