Merge pull request #4063 from randombit/jack/fix-4023

Fix several bugs relating to OID encoding and decoding

src/cli/asn1.cpp

@@ -102,7 +102,7 @@ class OID_Info final : public Command {
             }
 
             return;
-         } catch(Botan::Decoding_Error&) {}
+         } catch(Botan::Exception&) {}
 
          // This throws if the string is not known
          Botan::OID oid = Botan::OID::from_string(oid_str);

src/lib/asn1/asn1_obj.h

@@ -12,6 +12,7 @@
 #include <chrono>
 #include <iosfwd>
 #include <optional>
+#include <span>
 #include <string>
 #include <string_view>
 #include <unordered_map>
@@ -151,6 +152,8 @@ class BOTAN_PUBLIC_API(2, 0) BER_Object final {
 
       size_t length() const { return m_value.size(); }
 
+      std::span<const uint8_t> data() const { return std::span{m_value}; }
+
       void assert_is_a(ASN1_Type type_tag, ASN1_Class class_tag, std::string_view descr = "object") const;
 
       bool is_a(ASN1_Type type_tag, ASN1_Class class_tag) const;
@@ -219,23 +222,21 @@ class BOTAN_PUBLIC_API(2, 0) OID final : public ASN1_Object {
 
       /**
       * Construct an OID from a string.
-      * @param str a string in the form "a.b.c" etc., where a,b,c are numbers
+      * @param str a string in the form "a.b.c" etc., where a,b,c are integers
+      *
+      * Note: it is currently required that each integer fit into 32 bits
       */
       explicit OID(std::string_view str);
 
       /**
       * Initialize an OID from a sequence of integer values
       */
-      explicit OID(std::initializer_list<uint32_t> init) : m_id(init) {
-         BOTAN_ARG_CHECK(m_id.size() > 2 && m_id[0] <= 2 && (m_id[0] != 2 || m_id[1] <= 39), "Invalid OID");
-      }
+      explicit OID(std::initializer_list<uint32_t> init);
 
       /**
       * Initialize an OID from a vector of integer values
       */
-      explicit OID(std::vector<uint32_t>&& init) : m_id(init) {
-         BOTAN_ARG_CHECK(m_id.size() > 2 && m_id[0] <= 2 && (m_id[0] != 2 || m_id[1] <= 39), "Invalid OID");
-      }
+      BOTAN_DEPRECATED("Use another contructor") explicit OID(std::vector<uint32_t>&& init);
 
       /**
       * Construct an OID from a string.
@@ -268,15 +269,7 @@ class BOTAN_PUBLIC_API(2, 0) OID final : public ASN1_Object {
       * Find out whether this OID has a value
       * @return true is this OID has a value
       */
-      bool has_value() const { return (m_id.empty() == false); }
-
-      /**
-      * Get this OID as list (vector) of its components.
-      * @return vector representing this OID
-      */
-      const std::vector<uint32_t>& get_components() const { return m_id; }
-
-      const std::vector<uint32_t>& get_id() const { return get_components(); }
+      bool has_value() const { return !empty(); }
 
       /**
       * Get this OID as a dotted-decimal string
@@ -308,14 +301,28 @@ class BOTAN_PUBLIC_API(2, 0) OID final : public ASN1_Object {
       */
       bool operator==(const OID& other) const { return m_id == other.m_id; }
 
-   private:
-      std::unordered_map<std::string, std::string> load_oid2str_map();
-      std::unordered_map<std::string, OID> load_str2oid_map();
+      /**
+      * Get this OID as list (vector) of its components.
+      * @return vector representing this OID
+      */
+      BOTAN_DEPRECATED("Do not access the integer values, use eg to_string")
+      const std::vector<uint32_t>& get_components() const {
+         return m_id;
+      }
+
+      BOTAN_DEPRECATED("Do not access the integer values, use eg to_string")
+      const std::vector<uint32_t>& get_id() const {
+         return m_id;
+      }
 
+   private:
       std::vector<uint32_t> m_id;
 };
 
-std::ostream& operator<<(std::ostream& out, const OID& oid);
+inline std::ostream& operator<<(std::ostream& out, const OID& oid) {
+   out << oid.to_string();
+   return out;
+}
 
 /**
 * Compare two OIDs.

src/lib/asn1/asn1_oid.cpp

@@ -1,6 +1,6 @@
 /*
 * ASN.1 OID
-* (C) 1999-2007 Jack Lloyd
+* (C) 1999-2007,2024 Jack Lloyd
 *
 * Botan is released under the Simplified BSD License (see license.txt)
 */
@@ -11,15 +11,27 @@
 #include <botan/der_enc.h>
 #include <botan/internal/bit_ops.h>
 #include <botan/internal/fmt.h>
+#include <botan/internal/int_utils.h>
 #include <botan/internal/oid_map.h>
 #include <botan/internal/parsing.h>
+#include <botan/internal/stl_util.h>
 #include <algorithm>
+#include <span>
 #include <sstream>
 
 namespace Botan {
 
 namespace {
 
+void oid_valid_check(std::span<const uint32_t> oid) {
+   BOTAN_ARG_CHECK(oid.size() >= 2, "OID too short to be valid");
+   BOTAN_ARG_CHECK(oid[0] <= 2, "OID root out of range");
+   BOTAN_ARG_CHECK(oid[1] <= 39 || oid[0] == 2, "OID second arc too large");
+   // This last is a limitation of using 32 bit integers when decoding
+   // not a limitation of ASN.1 object identifiers in general
+   BOTAN_ARG_CHECK(oid[1] <= 0xFFFFFFAF, "OID second arc too large");
+}
+
 // returns empty on invalid
 std::vector<uint32_t> parse_oid_str(std::string_view oid) {
    try {
@@ -43,8 +55,8 @@ std::vector<uint32_t> parse_oid_str(std::string_view oid) {
       }
 
       return oid_elems;
-   } catch(Invalid_Argument&)  // thrown by to_u32bit
-   {
+   } catch(Invalid_Argument&) {
+      // thrown by to_u32bit
       return std::vector<uint32_t>();
    }
 }
@@ -81,24 +93,29 @@ OID OID::from_string(std::string_view str) {
       return o;
    }
 
-   std::vector<uint32_t> raw = parse_oid_str(str);
-
-   if(!raw.empty()) {
-      return OID(std::move(raw));
-   }
+   // Try to parse as a dotted decimal
+   try {
+      return OID(str);
+   } catch(...) {}
 
    throw Lookup_Error(fmt("No OID associated with name '{}'", str));
 }
 
+OID::OID(std::initializer_list<uint32_t> init) : m_id(init) {
+   oid_valid_check(m_id);
+}
+
+OID::OID(std::vector<uint32_t>&& init) : m_id(std::move(init)) {
+   oid_valid_check(m_id);
+}
+
 /*
 * ASN.1 OID Constructor
 */
 OID::OID(std::string_view oid_str) {
    if(!oid_str.empty()) {
       m_id = parse_oid_str(oid_str);
-      if(m_id.size() < 2 || m_id[0] > 2 || (m_id[0] < 2 && m_id[1] > 39)) {
-         throw Decoding_Error(fmt("Invalid OID '{}'", oid_str));
-      }
+      oid_valid_check(m_id);
    }
 }
 
@@ -107,7 +124,15 @@ OID::OID(std::string_view oid_str) {
 */
 std::string OID::to_string() const {
    std::ostringstream out;
-   out << (*this);
+
+   for(size_t i = 0; i != m_id.size(); ++i) {
+      // avoid locale issues with integer formatting
+      out << std::to_string(m_id[i]);
+      if(i != m_id.size() - 1) {
+         out << ".";
+      }
+   }
+
    return out.str();
 }
 
@@ -137,20 +162,6 @@ bool operator<(const OID& a, const OID& b) {
    return std::lexicographical_compare(oid1.begin(), oid1.end(), oid2.begin(), oid2.end());
 }
 
-std::ostream& operator<<(std::ostream& out, const OID& oid) {
-   const auto& val = oid.get_components();
-
-   for(size_t i = 0; i != val.size(); ++i) {
-      // avoid locale issues with integer formatting
-      out << std::to_string(val[i]);
-      if(i != val.size() - 1) {
-         out << ".";
-      }
-   }
-
-   return out;
-}
-
 /*
 * DER encode an OBJECT IDENTIFIER
 */
@@ -159,28 +170,33 @@ void OID::encode_into(DER_Encoder& der) const {
       throw Invalid_Argument("OID::encode_into: OID is invalid");
    }
 
-   std::vector<uint8_t> encoding;
-
-   if(m_id[0] > 2 || m_id[1] >= 40) {
-      throw Encoding_Error("Invalid OID prefix, cannot encode");
-   }
-
-   encoding.push_back(static_cast<uint8_t>(40 * m_id[0] + m_id[1]));
-
-   for(size_t i = 2; i != m_id.size(); ++i) {
-      if(m_id[i] == 0) {
-         encoding.push_back(0);
+   auto append = [](std::vector<uint8_t>& encoding, uint32_t z) {
+      if(z <= 0x7F) {
+         encoding.push_back(static_cast<uint8_t>(z));
       } else {
-         size_t blocks = high_bit(m_id[i]) + 6;
-         blocks = (blocks - (blocks % 7)) / 7;
+         size_t z7 = (high_bit(z) + 7 - 1) / 7;
 
-         BOTAN_ASSERT(blocks > 0, "Math works");
+         for(size_t j = 0; j != z7; ++j) {
+            uint8_t zp = static_cast<uint8_t>(z >> (7 * (z7 - j - 1)) & 0x7F);
 
-         for(size_t j = 0; j != blocks - 1; ++j) {
-            encoding.push_back(0x80 | ((m_id[i] >> 7 * (blocks - j - 1)) & 0x7F));
+            if(j != z7 - 1) {
+               zp |= 0x80;
+            }
+
+            encoding.push_back(zp);
          }
-         encoding.push_back(m_id[i] & 0x7F);
       }
+   };
+
+   std::vector<uint8_t> encoding;
+
+   // We know 40 * root can't overflow because root is between 0 and 2
+   auto first = BOTAN_ASSERT_IS_SOME(checked_add(40 * m_id[0], m_id[1]));
+
+   append(encoding, first);
+
+   for(size_t i = 2; i != m_id.size(); ++i) {
+      append(encoding, m_id[i]);
    }
    der.add_object(ASN1_Type::ObjectId, ASN1_Class::Universal, encoding);
 }
@@ -194,35 +210,75 @@ void OID::decode_from(BER_Decoder& decoder) {
       throw BER_Bad_Tag("Error decoding OID, unknown tag", obj.tagging());
    }
 
-   const size_t length = obj.length();
-   const uint8_t* bits = obj.bits();
-
-   if(length < 2 && !(length == 1 && bits[0] == 0)) {
+   if(obj.length() == 0) {
       throw BER_Decoding_Error("OID encoding is too short");
    }
 
-   m_id.clear();
-   m_id.push_back(bits[0] / 40);
-   m_id.push_back(bits[0] % 40);
+   auto consume = [](BufferSlicer& data) -> uint32_t {
+      BOTAN_ASSERT_NOMSG(!data.empty());
+      uint32_t b = data.take_byte();
 
-   size_t i = 0;
-   while(i != length - 1) {
-      uint32_t component = 0;
-      while(i != length - 1) {
-         ++i;
+      if(b > 0x7F) {
+         b &= 0x7F;
 
-         if(component >> (32 - 7)) {
-            throw Decoding_Error("OID component overflow");
+         // Even BER requires that the OID have minimal length, ie that
+         // the first byte of a multibyte encoding cannot be zero
+         // See X.690 section 8.19.2
+         if(b == 0) {
+            throw Decoding_Error("Leading zero byte in multibyte OID encoding");
          }
 
-         component = (component << 7) + (bits[i] & 0x7F);
+         while(true) {
+            if(data.empty()) {
+               throw Decoding_Error("Truncated OID value");
+            }
+
+            const uint8_t next = data.take_byte();
+            const bool more = (next & 0x80);
+            const uint8_t value = next & 0x7F;
 
-         if(!(bits[i] & 0x80)) {
-            break;
+            if((b >> (32 - 7)) != 0) {
+               throw Decoding_Error("OID component overflow");
+            }
+
+            b = (b << 7) | value;
+
+            if(!more) {
+               break;
+            }
          }
       }
-      m_id.push_back(component);
+
+      return b;
+   };
+
+   BufferSlicer data(obj.data());
+   std::vector<uint32_t> parts;
+   while(!data.empty()) {
+      const uint32_t comp = consume(data);
+
+      if(parts.empty()) {
+         // divide into root and second arc
+
+         const uint32_t root_arc = [](uint32_t b0) -> uint32_t {
+            if(b0 < 40) {
+               return 0;
+            } else if(b0 < 80) {
+               return 1;
+            } else {
+               return 2;
+            }
+         }(comp);
+
+         parts.push_back(root_arc);
+         BOTAN_ASSERT_NOMSG(comp >= 40 * root_arc);
+         parts.push_back(comp - 40 * root_arc);
+      } else {
+         parts.push_back(comp);
+      }
    }
+
+   m_id = parts;
 }
 
 }  // namespace Botan