Fix X509 DN comparisons

CVE-2017-2801

5 files changed, 84 insertions, 1 deletions

diff --git a/doc/security.rst b/doc/security.rst
index 9ed29ef03..9d08ed8c8 100644
--- a/doc/security.rst
+++ b/doc/security.rst
@@ -18,7 +18,18 @@ https://keybase.io/jacklloyd and on most PGP keyservers.
 2017
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-* 2017-03-23 (CVE-2016-7252): Incorrect bcrypt computation
+* 2017-04-04 (CVE-2017-2801): Incorrect comparison in X.509 DN strings
+
+  Botan's implementation of X.509 name comparisons had a flaw which
+  could result in an out of bound memory read while processing a
+  specially formed DN. This could potentially be exploited for
+  information disclosure or denial of service, or result in incorrect
+  validation results. Found independently by Aleksandar Nikolic of
+  Cisco Talos, and OSS-Fuzz automated fuzzing infrastructure.
+
+  Bug introduced in 1.6.0 or earlier, fixed in 2.1.0 and 1.10.16
+
+* 2017-03-23 (CVE-2017-7252): Incorrect bcrypt computation
 
   Botan's implementation of bcrypt password hashing scheme truncated long
   passwords at 56 characters, instead of at bcrypt's standard 72 characters
diff --git a/src/lib/utils/parsing.cpp b/src/lib/utils/parsing.cpp
index 8fd2ccc52..cd0c2409e 100644
--- a/src/lib/utils/parsing.cpp
+++ b/src/lib/utils/parsing.cpp
@@ -239,6 +239,8 @@ bool x500_name_cmp(const std::string& name1, const std::string& name2)
 
          if(p1 == name1.end() && p2 == name2.end())
             return true;
+         if(p1 == name1.end() || p2 == name2.end())
+            return false;
          }
 
       if(!Charset::caseless_cmp(*p1, *p2))
diff --git a/src/tests/data/fuzz/x509/cve_2017_2801.der b/src/tests/data/fuzz/x509/cve_2017_2801.der
new file mode 100644
index 000000000..fe615d2c4
--- /dev/null
+++ b/src/tests/data/fuzz/x509/cve_2017_2801.der
diff --git a/src/tests/data/x509_dn.vec b/src/tests/data/x509_dn.vec
new file mode 100644
index 000000000..ab9e2dec3
--- /dev/null
+++ b/src/tests/data/x509_dn.vec
@@ -0,0 +1,14 @@
+
+[Equal]
+DN1 = 301C310B3009060355040613025654310D300B0603550403130454455354
+DN2 = 301C310B3009060355040613025654310D300B0603550403130454455354
+
+[Unequal]
+DN1 = 301C310B3009060355040613025654310D300B0603550403130454450054
+DN2 = 301C310B3009060355040613025600310D300B0603550403130454455354
+
+DN1 = 3019311730150603550403140E4141200141414141414141414141
+DN2 = 3019311730150603550403130E4141202020202020202020202020
+
+DN1 = 3018311630140603550403130D41412041414141414141414141
+DN2 = 3019311730150603550403130E4141202020202020202020202020
diff --git a/src/tests/test_x509_dn.cpp b/src/tests/test_x509_dn.cpp
new file mode 100644
index 000000000..3a2fdf7d0
--- /dev/null
+++ b/src/tests/test_x509_dn.cpp
@@ -0,0 +1,56 @@
+/*
+* (C) 2017 Jack Lloyd
+*
+* Botan is released under the Simplified BSD License (see license.txt)
+*/
+
+#include "tests.h"
+
+#if defined(BOTAN_HAS_ASN1)
+  #include <botan/x509_dn.h>
+  #include <botan/ber_dec.h>
+#endif
+
+namespace Botan_Tests {
+
+#if defined(BOTAN_HAS_ASN1)
+class X509_DN_Comparisons_Tests : public Text_Based_Test
+   {
+   public:
+      X509_DN_Comparisons_Tests() :
+         Text_Based_Test("x509_dn.vec", "DN1,DN2")
+         {}
+
+      Test::Result run_one_test(const std::string& type, const VarMap& vars) override
+         {
+         const std::vector<uint8_t> dn_bits1 = get_req_bin(vars, "DN1");
+         const std::vector<uint8_t> dn_bits2 = get_req_bin(vars, "DN2");
+         const bool dn_same = (type == "Equal");
+
+         Test::Result result("X509_DN comparisons");
+         try
+            {
+            Botan::X509_DN dn1;
+            Botan::BER_Decoder bd1(dn_bits1);
+            dn1.decode_from(bd1);
+
+            Botan::X509_DN dn2;
+            Botan::BER_Decoder bd2(dn_bits2);
+            dn2.decode_from(bd2);
+
+            const bool compared_same = (dn1 == dn2);
+            result.test_eq("Comparison matches expected", dn_same, compared_same);
+            }
+         catch(Botan::Exception& e)
+            {
+            result.test_failure(e.what());
+            }
+
+         return result;
+         }
+   };
+
+BOTAN_REGISTER_TEST("x509_dn_cmp", X509_DN_Comparisons_Tests);
+#endif
+
+}