Fix memory leak in ASN.1

Affects 1.1.0 dev branch only; introduced around commit f93ad22 Found with LibFuzzer
openssl · ekasper · Mar 30, 2016 · Mar 31, 2016 · Mar 31, 2016 · dd5ac557f052cc2b7f718ac44a8cb7ac6f77dca8
commit dd5ac557f052cc2b7f718ac44a8cb7ac6f77dca8
diff --git a/crypto/asn1/tasn_dec.c b/crypto/asn1/tasn_dec.c
@@ -617,6 +617,8 @@ static int asn1_template_noexp_d2i(ASN1_VALUE **val,
                                      ASN1_ITEM_ptr(tt->item), -1, 0, 0, ctx)) {
                 ASN1err(ASN1_F_ASN1_TEMPLATE_NOEXP_D2I,
                         ERR_R_NESTED_ASN1_ERROR);
+                /* |skfield| may be partially allocated despite failure. */
+                ASN1_item_free(skfield, ASN1_ITEM_ptr(tt->item));
                 goto err;
             }
             len -= p - q;

diff --git a/test/Makefile.in b/test/Makefile.in
@@ -84,6 +84,7 @@ DTLSV1LISTENTEST = dtlsv1listentest
 CTTEST=	ct_test
 THREADSTEST=	threadstest
 AFALGTEST= afalgtest
+D2ITEST = d2i_test
 
 TESTS=		alltests
 
@@ -106,7 +107,7 @@ EXE=	$(NPTEST)$(EXE_EXT) $(MEMLEAKTEST)$(EXE_EXT) \
 	$(CONSTTIMETEST)$(EXE_EXT) $(VERIFYEXTRATEST)$(EXE_EXT) \
 	$(CLIENTHELLOTEST)$(EXE_EXT) $(PACKETTEST)$(EXE_EXT) $(ASYNCTEST)$(EXE_EXT) \
 	$(DTLSV1LISTENTEST)$(EXE_EXT) $(CTTEST)$(EXE_EXT) $(THREADSTEST)$(EXE_EXT) \
-	$(AFALGTEST)$(EXE_EXT)
+	$(AFALGTEST)$(EXE_EXT) $(D2ITEST)$(EXE_EXT)
 
 # $(METHTEST)$(EXE_EXT)
 
@@ -124,7 +125,7 @@ OBJ=	$(NPTEST).o $(MEMLEAKTEST).o \
 	$(HEARTBEATTEST).o $(P5_CRPT2_TEST).o \
 	$(CONSTTIMETEST).o $(VERIFYEXTRATEST).o $(CLIENTHELLOTEST).o \
 	$(PACKETTEST).o $(ASYNCTEST).o $(DTLSV1LISTENTEST).o $(CTTEST).o \
-	$(THREADSTEST).o testutil.o $(AFALGTEST).o
+	$(THREADSTEST).o testutil.o $(AFALGTEST).o $(D2ITEST).o
 
 SRC=	$(NPTEST).c $(MEMLEAKTEST).c \
 	$(BNTEST).c $(ECTEST).c \
@@ -139,7 +140,7 @@ SRC=	$(NPTEST).c $(MEMLEAKTEST).c \
 	$(HEARTBEATTEST).c $(P5_CRPT2_TEST).c \
 	$(CONSTTIMETEST).c $(VERIFYEXTRATEST).c $(CLIENTHELLOTEST).c \
 	$(PACKETTEST).c $(ASYNCTEST).c $(DTLSV1LISTENTEST).c $(CTTEST).c \
-	$(THREADSTEST).c testutil.c $(AFALGTEST).c
+	$(THREADSTEST).c testutil.c $(AFALGTEST).c $(D2ITEST).c
 
 HEADER=	testutil.h
 
@@ -385,4 +386,7 @@ dummytest$(EXE_EXT): dummytest.o $(DLIBCRYPTO)
 $(AFALGTEST)$(EXE_EXT): $(AFALGTEST).o $(DLIBCRYPTO)
 	@target=$(AFALGTEST); $(BUILD_CMD)
 
+$(D2ITEST)$(EXE_EXT): $(D2ITEST).o $(DLIBCRYPTO) testutil.o
+	@target=$(D2ITEST) testutil=testutil.o; $(BUILD_CMD)
+
 # DO NOT DELETE THIS LINE -- make depend depends on it.
diff --git a/test/build.info b/test/build.info
@@ -14,7 +14,7 @@ PROGRAMS=\
         danetest heartbeat_test p5_crpt2_test \
         constant_time_test verify_extra_test clienthellotest \
         packettest asynctest secmemtest srptest memleaktest \
-        dtlsv1listentest ct_test threadstest afalgtest
+        dtlsv1listentest ct_test threadstest afalgtest d2i_test
 
 SOURCE[aborttest]=aborttest.c
 INCLUDE[aborttest]={- rel2abs(catdir($builddir,"../include")) -} ../include
@@ -220,4 +220,8 @@ SOURCE[afalgtest]=afalgtest.c
 INCLUDE[afalgtest]={- rel2abs(catdir($builddir,"../include")) -} .. ../include
 DEPEND[afalgtest]=../libcrypto
 
+SOURCE[d2i_test]=d2i_test.c testutil.c
+INCLUDE[d2i_test]={- rel2abs(catdir($builddir,"../include")) -} .. ../include
+DEPEND[d2i_test]=../libcrypto
+
 INCLUDE[testutil.o]=..
diff --git a/test/d2i-tests/bad_cert.der b/test/d2i-tests/bad_cert.der
diff --git a/test/d2i_test.c b/test/d2i_test.c
@@ -0,0 +1,87 @@
+/*
+ * Copyright 2016 The OpenSSL Project Authors. All Rights Reserved.
+ *
+ * Licensed under the OpenSSL licenses, (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * https://www.openssl.org/source/license.html
+ * or in the file LICENSE in the source distribution.
+ */
+
+/* Regression tests for ASN.1 parsing bugs. */
+
+#include <stdio.h>
+
+#include "testutil.h"
+
+#include <openssl/bio.h>
+#include <openssl/err.h>
+#include <openssl/x509.h>
+
+static const char *test_file;
+
+typedef struct d2i_test_fixture {
+    const char *test_case_name;
+} D2I_TEST_FIXTURE;
+
+
+static D2I_TEST_FIXTURE set_up(const char *const test_case_name)
+{
+    D2I_TEST_FIXTURE fixture;
+    fixture.test_case_name = test_case_name;
+    return fixture;
+}
+
+static int execute_test(D2I_TEST_FIXTURE fixture)
+{
+    BIO *bio = NULL;
+    X509 *x509 = NULL;
+    int ret = 1;
+
+    if ((bio = BIO_new_file(test_file, "r")) == NULL)
+        return 1;
+
+    x509 = d2i_X509_bio(bio, NULL);
+    if (x509 != NULL)
+        goto err;
+
+    ret = 0;
+
+ err:
+    BIO_free(bio);
+    X509_free(x509);
+    return ret;
+}
+
+static void tear_down(D2I_TEST_FIXTURE fixture)
+{
+    ERR_print_errors_fp(stderr);
+}
+
+#define SETUP_D2I_TEST_FIXTURE() \
+    SETUP_TEST_FIXTURE(D2I_TEST_FIXTURE, set_up)
+
+#define EXECUTE_D2I_TEST() \
+    EXECUTE_TEST(execute_test, tear_down)
+
+static int test_bad_certificate()
+{
+    SETUP_D2I_TEST_FIXTURE();
+    EXECUTE_D2I_TEST();
+}
+
+int main(int argc, char **argv)
+{
+    int result = 0;
+
+    if (argc != 2)
+        return 1;
+
+    test_file = argv[1];
+
+    ADD_TEST(test_bad_certificate);
+
+    result = run_tests(argv[0]);
+
+    return result;
+}
diff --git a/test/recipes/25-test_d2i.t b/test/recipes/25-test_d2i.t
@@ -0,0 +1,14 @@
+#! /usr/bin/perl
+
+use strict;
+use warnings;
+
+use File::Spec;
+use OpenSSL::Test qw/:DEFAULT srctop_file/;
+
+setup("test_d2i");
+
+plan tests => 1;
+
+ok(run(test(["d2i_test", srctop_file('test','d2i-tests','bad_cert.der')])),
+   "Running d2i_test bad_cert.der");