Fix a crash in reuse of i2d_X509_PUBKEY

If the second PUBKEY is malformed there is use after free.

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8122)

diff --git a/crypto/x509/x_pubkey.c b/crypto/x509/x_pubkey.c
index f980af77f657e4de30eddeac11677fb219b81c87..be42684e44b131275417c4325a57392283e5aec0 100644 (file)
--- a/crypto/x509/x_pubkey.c
+++ b/crypto/x509/x_pubkey.c
@@ -36,6 +36,7 @@ static int pubkey_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it,
         /* Attempt to decode public key and cache in pubkey structure. */
         X509_PUBKEY *pubkey = (X509_PUBKEY *)*pval;
         EVP_PKEY_free(pubkey->pkey);
+        pubkey->pkey = NULL;
         /*
          * Opportunistically decode the key but remove any non fatal errors
          * from the queue. Subsequent explicit attempts to decode/use the key

diff --git a/test/evp_extra_test.c b/test/evp_extra_test.c
index eefebd586de27d3687245bf76fc3fa1c70af0d81..eac0c43ba10db3a504c7ef0bf8ca0745187336a5 100644 (file)
--- a/test/evp_extra_test.c
+++ b/test/evp_extra_test.c
@@ -299,6 +299,21 @@ static const unsigned char kExampleECPubKeyDER[] = {
     0x56, 0x6a, 0xc6, 0xc8, 0xa5, 0x0b, 0xe5
 };
 
+/*
+ * kExampleBadECKeyDER is a sample EC public key with a wrong OID
+ * 1.2.840.10045.2.2 instead of 1.2.840.10045.2.1 - EC Public Key
+ */
+static const unsigned char kExampleBadECPubKeyDER[] = {
+    0x30, 0x59, 0x30, 0x13, 0x06, 0x07, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02,
+    0x02, 0x06, 0x08, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x03, 0x01, 0x07, 0x03,
+    0x42, 0x00, 0x04, 0xba, 0xeb, 0x83, 0xfb, 0x3b, 0xb2, 0xff, 0x30, 0x53,
+    0xdb, 0xce, 0x32, 0xf2, 0xac, 0xae, 0x44, 0x0d, 0x3d, 0x13, 0x53, 0xb8,
+    0xd1, 0x68, 0x55, 0xde, 0x44, 0x46, 0x05, 0xa6, 0xc9, 0xd2, 0x04, 0xb7,
+    0xe3, 0xa2, 0x96, 0xc8, 0xb2, 0x5e, 0x22, 0x03, 0xd7, 0x03, 0x7a, 0x8b,
+    0x13, 0x5c, 0x42, 0x49, 0xc2, 0xab, 0x86, 0xd6, 0xac, 0x6b, 0x93, 0x20,
+    0x56, 0x6a, 0xc6, 0xc8, 0xa5, 0x0b, 0xe5
+};
+
 static const unsigned char pExampleECParamDER[] = {
     0x06, 0x08, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x03, 0x01, 0x07
 };
@@ -963,6 +978,37 @@ static int test_HKDF(void)
     return ret;
 }
 
+#ifndef OPENSSL_NO_EC
+static int test_X509_PUBKEY_inplace(void)
+{
+  int ret = 0;
+  X509_PUBKEY *xp = NULL;
+  const unsigned char *p = kExampleECPubKeyDER;
+  size_t input_len = sizeof(kExampleECPubKeyDER);
+
+  if (!TEST_ptr(xp = d2i_X509_PUBKEY(NULL, &p, input_len)))
+    goto done;
+
+  if (!TEST_ptr(X509_PUBKEY_get0(xp)))
+    goto done;
+
+  p = kExampleBadECPubKeyDER;
+  input_len = sizeof(kExampleBadECPubKeyDER);
+
+  if (!TEST_ptr(xp = d2i_X509_PUBKEY(&xp, &p, input_len)))
+    goto done;
+
+  if (!TEST_true(X509_PUBKEY_get0(xp) == NULL))
+    goto done;
+
+  ret = 1;
+
+done:
+  X509_PUBKEY_free(xp);
+  return ret;
+}
+#endif
+
 int setup_tests(void)
 {
     ADD_TEST(test_EVP_DigestSignInit);
@@ -987,5 +1033,8 @@ int setup_tests(void)
         return 0;
     ADD_ALL_TESTS(test_EVP_PKEY_check, OSSL_NELEM(keycheckdata));
     ADD_TEST(test_HKDF);
+#ifndef OPENSSL_NO_EC
+    ADD_TEST(test_X509_PUBKEY_inplace);
+#endif
     return 1;
 }