Fix second memory leak

openssl · ekasper · Mar 30, 2016 · Mar 31, 2016 · Mar 31, 2016 · 19b5b9194071d1d84e38ac9a952e715afbc85a81
commit 19b5b9194071d1d84e38ac9a952e715afbc85a81
diff --git 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;

diff --git a/test/d2i-tests/bad_generalname.der b/test/d2i-tests/bad_generalname.der
@@ -0,0 +1 @@
+��0;�!;)''��!l�(,:��(*;�:���:�*�*;i)*w*�)�;U:'):�;l*!'ң
diff --git a/test/d2i_test.c b/test/d2i_test.c
@@ -14,10 +14,13 @@
 
 #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 {
@@ -35,21 +38,32 @@ static D2I_TEST_FIXTURE set_up(const char *const test_case_name)
 static int execute_test(D2I_TEST_FIXTURE fixture)
 {
  BIO *bio = NULL;
- X509 *x509 = NULL;
+ ASN1_VALUE *value = NULL;
  int ret = 1;
+ unsigned char buf[2048];
+ const unsigned char *buf_ptr = buf;
+ size_t len;
 
  if ((bio = BIO_new_file(test_file, "r")) == NULL)
  return 1;
 
- x509 = d2i_X509_bio(bio, NULL);
- if (x509 != NULL)
+ /*
+ * 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);
- X509_free(x509);
+ ASN1_item_free(value, item_type);
  return ret;
 }
 
@@ -64,22 +78,37 @@ static void tear_down(D2I_TEST_FIXTURE fixture)
 #define EXECUTE_D2I_TEST() \
  EXECUTE_TEST(execute_test, tear_down)
 
-static int test_bad_certificate()
+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 != 2)
+ if (argc != 3)
  return 1;
 
- test_file = argv[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");
+ return 1;
+ }
 
- ADD_TEST(test_bad_certificate);
+ ADD_TEST(test_bad_asn1);
 
  result = run_tests(argv[0]);
 

diff --git a/test/recipes/25-test_d2i.t b/test/recipes/25-test_d2i.t
@@ -8,7 +8,12 @@ use OpenSSL::Test qw/:DEFAULT srctop_file/;
 
 setup("test_d2i");
 
-plan tests => 1;
+plan tests => 2;
 
-ok(run(test(["d2i_test", srctop_file('test','d2i-tests','bad_cert.der')])),
+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");