[openssl-commits] [openssl] master update
Emilia Kasper
emilia at openssl.org
Mon Apr 4 11:26:27 UTC 2016
The branch master has been updated
via 1400f013e10c8ec624947d9187bebb20274385dc (commit)
from b5851bbc43dcf497189d16395dfa925d1562ee06 (commit)
- Log -----------------------------------------------------------------
commit 1400f013e10c8ec624947d9187bebb20274385dc
Author: Emilia Kasper <emilia at openssl.org>
Date: Wed Mar 30 22:37:05 2016 +0200
Fix memory leaks in ASN.1
These leaks affect 1.1.0 dev branch only; introduced around commit
f93ad22f6adb00e722c130e792799467f3927b56
Found with LibFuzzer
Reviewed-by: Ben Laurie <ben at openssl.org>
-----------------------------------------------------------------------
Summary of changes:
crypto/asn1/tasn_dec.c | 9 ++-
test/Makefile.in | 10 +++-
test/build.info | 6 +-
test/d2i-tests/bad_cert.der | Bin 0 -> 1007 bytes
test/d2i-tests/bad_generalname.der | 1 +
test/d2i_test.c | 117 +++++++++++++++++++++++++++++++++++++
test/recipes/25-test_d2i.t | 19 ++++++
7 files changed, 157 insertions(+), 5 deletions(-)
create mode 100644 test/d2i-tests/bad_cert.der
create mode 100644 test/d2i-tests/bad_generalname.der
create mode 100644 test/d2i_test.c
create mode 100644 test/recipes/25-test_d2i.t
diff --git a/crypto/asn1/tasn_dec.c b/crypto/asn1/tasn_dec.c
index b025e58..5715921 100644
--- a/crypto/asn1/tasn_dec.c
+++ b/crypto/asn1/tasn_dec.c
@@ -273,6 +273,12 @@ static int asn1_item_embed_d2i(ASN1_VALUE **pval, const unsigned char **in,
/* If field not present, try the next one */
if (ret == -1)
continue;
+ /*
+ * Set the choice selector here to ensure that the value is
+ * correctly freed upon error. It may be partially initialized
+ * even if parsing failed.
+ */
+ asn1_set_choice_selector(pval, i, it);
/* If positive return, read OK, break loop */
if (ret > 0)
break;
@@ -294,7 +300,6 @@ static int asn1_item_embed_d2i(ASN1_VALUE **pval, const unsigned char **in,
goto err;
}
- asn1_set_choice_selector(pval, i, it);
if (asn1_cb && !asn1_cb(ASN1_OP_D2I_POST, pval, it, NULL))
goto auxerr;
*in = p;
@@ -617,6 +622,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
index e10af0b..ee66729 100644
--- 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
index 74f83a3..083412c 100644
--- 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
new file mode 100644
index 0000000..f75efad
Binary files /dev/null and b/test/d2i-tests/bad_cert.der differ
diff --git a/test/d2i-tests/bad_generalname.der b/test/d2i-tests/bad_generalname.der
new file mode 100644
index 0000000..af45855
--- /dev/null
+++ b/test/d2i-tests/bad_generalname.der
@@ -0,0 +1 @@
+¥€0;¶!;)''ï÷!l¿(,:µ¿(*;©:§«½:“**;i)*w*ë)ã;U:'):ñ;l*!'Ò£
\ No newline at end of file
diff --git a/test/d2i_test.c b/test/d2i_test.c
new file mode 100644
index 0000000..cf01012
--- /dev/null
+++ b/test/d2i_test.c
@@ -0,0 +1,117 @@
+/*
+ * 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 <string.h>
+
+#include "testutil.h"
+
+#include <openssl/asn1.h>
+#include <openssl/bio.h>
+#include <openssl/err.h>
+#include <openssl/x509.h>
+#include <openssl/x509v3.h>
+
+static const ASN1_ITEM *item_type;
+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;
+ ASN1_VALUE *value = NULL;
+ int ret = 1;
+ unsigned char buf[2048];
+ const unsigned char *buf_ptr = buf;
+ int len;
+
+ if ((bio = BIO_new_file(test_file, "r")) == NULL)
+ return 1;
+
+ /*
+ * We don't use ASN1_item_d2i_bio because it, apparently,
+ * errors too early for some inputs.
+ */
+ len = BIO_read(bio, buf, sizeof buf);
+ if (len < 0)
+ goto err;
+
+ value = ASN1_item_d2i(NULL, &buf_ptr, len, item_type);
+ if (value != NULL)
+ goto err;
+
+ ret = 0;
+
+ err:
+ BIO_free(bio);
+ ASN1_item_free(value, item_type);
+ 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_asn1()
+{
+ SETUP_D2I_TEST_FIXTURE();
+ EXECUTE_D2I_TEST();
+}
+
+/*
+ * Usage: d2i_test <type> <file>, e.g.
+ * d2i_test generalname bad_generalname.der
+ */
+int main(int argc, char **argv)
+{
+ int result = 0;
+ const char *test_type_name;
+
+ if (argc != 3)
+ return 1;
+
+ test_type_name = argv[1];
+ test_file = argv[2];
+
+ if (strcmp(test_type_name, "generalname") == 0) {
+ item_type = ASN1_ITEM_rptr(GENERAL_NAME);
+ } else if (strcmp(test_type_name, "x509") == 0) {
+ item_type = ASN1_ITEM_rptr(X509);
+ } else {
+ fprintf(stderr, "Bad type %s\n", test_type_name);
+ return 1;
+ }
+
+ ADD_TEST(test_bad_asn1);
+
+ result = run_tests(argv[0]);
+
+ return result;
+}
diff --git a/test/recipes/25-test_d2i.t b/test/recipes/25-test_d2i.t
new file mode 100644
index 0000000..a9c259d
--- /dev/null
+++ b/test/recipes/25-test_d2i.t
@@ -0,0 +1,19 @@
+#! /usr/bin/perl
+
+use strict;
+use warnings;
+
+use File::Spec;
+use OpenSSL::Test qw/:DEFAULT srctop_file/;
+
+setup("test_d2i");
+
+plan tests => 2;
+
+ok(run(test(["d2i_test", "x509",
+ srctop_file('test','d2i-tests','bad_cert.der')])),
+ "Running d2i_test bad_cert.der");
+
+ok(run(test(["d2i_test", "generalname",
+ srctop_file('test','d2i-tests','bad_generalname.der')])),
+ "Running d2i_test bad_generalname.der");
More information about the openssl-commits
mailing list