The branch OpenSSL_1_1_1-stable has been updated via 7c65179ad95d0f6f598ee82e763fce2567fe5802 (commit) via 65b88a75921533ada8b465bc8d5c0817ad927947 (commit) from 513ead860853e0d07f7fc43bf35d1b90fdad5a11 (commit)
- Log ----------------------------------------------------------------- commit 7c65179ad95d0f6f598ee82e763fce2567fe5802 Author: Shane Lontis <shane.lon...@oracle.com> Date: Wed Apr 21 13:49:29 2021 +1000 Test that we don't have a memory leak in d2i_ASN1_OBJECT. Fixes #14667 Reworked test supplied by @smcpeak into a unit test. Reviewed-by: Richard Levitte <levi...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/14938) commit 65b88a75921533ada8b465bc8d5c0817ad927947 Author: Richard Levitte <levi...@openssl.org> Date: Tue Apr 20 08:43:30 2021 +0200 ASN1: Ensure that d2i_ASN1_OBJECT() frees the strings on ASN1_OBJECT reuse The 'sn' and 'ln' strings may be dynamically allocated, and the ASN1_OBJECT flags have a bit set to say this. If an ASN1_OBJECT with such strings is passed to d2i_ASN1_OBJECT() for reuse, the strings must be freed, or there is a memory leak. Fixes #14667 Reviewed-by: Shane Lontis <shane.lon...@oracle.com> (Merged from https://github.com/openssl/openssl/pull/14938) ----------------------------------------------------------------------- Summary of changes: crypto/asn1/a_object.c | 13 ++++++++----- test/asn1_decode_test.c | 26 ++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/crypto/asn1/a_object.c b/crypto/asn1/a_object.c index d67a723c96..8790be340a 100644 --- a/crypto/asn1/a_object.c +++ b/crypto/asn1/a_object.c @@ -286,16 +286,13 @@ ASN1_OBJECT *c2i_ASN1_OBJECT(ASN1_OBJECT **a, const unsigned char **pp, } } - /* - * only the ASN1_OBJECTs from the 'table' will have values for ->sn or - * ->ln - */ if ((a == NULL) || ((*a) == NULL) || !((*a)->flags & ASN1_OBJECT_FLAG_DYNAMIC)) { if ((ret = ASN1_OBJECT_new()) == NULL) return NULL; - } else + } else { ret = (*a); + } p = *pp; /* detach data from object */ @@ -313,6 +310,12 @@ ASN1_OBJECT *c2i_ASN1_OBJECT(ASN1_OBJECT **a, const unsigned char **pp, ret->flags |= ASN1_OBJECT_FLAG_DYNAMIC_DATA; } memcpy(data, p, length); + /* If there are dynamic strings, free them here, and clear the flag */ + if ((ret->flags & ASN1_OBJECT_FLAG_DYNAMIC_STRINGS) != 0) { + OPENSSL_free((char *)ret->sn); + OPENSSL_free((char *)ret->ln); + ret->flags &= ~ASN1_OBJECT_FLAG_DYNAMIC_STRINGS; + } /* reattach data to object, after which it remains const */ ret->data = data; ret->length = length; diff --git a/test/asn1_decode_test.c b/test/asn1_decode_test.c index 18f0ca12e9..de818ab12e 100644 --- a/test/asn1_decode_test.c +++ b/test/asn1_decode_test.c @@ -12,6 +12,7 @@ #include <openssl/rand.h> #include <openssl/asn1t.h> +#include <openssl/obj_mac.h> #include "internal/numbers.h" #include "testutil.h" @@ -195,6 +196,30 @@ static int test_invalid_template(void) return 0; } +static int test_reuse_asn1_object(void) +{ + static unsigned char cn_der[] = { 0x06, 0x03, 0x55, 0x04, 0x06 }; + static unsigned char oid_der[] = { + 0x06, 0x06, 0x2a, 0x03, 0x04, 0x05, 0x06, 0x07 + }; + int ret = 0; + ASN1_OBJECT *obj; + unsigned char const *p = oid_der; + + /* Create an object that owns dynamically allocated 'sn' and 'ln' fields */ + + if (!TEST_ptr(obj = ASN1_OBJECT_create(NID_undef, cn_der, sizeof(cn_der), + "C", "countryName"))) + goto err; + /* reuse obj - this should not leak sn and ln */ + if (!TEST_ptr(d2i_ASN1_OBJECT(&obj, &p, sizeof(oid_der)))) + goto err; + ret = 1; +err: + ASN1_OBJECT_free(obj); + return ret; +} + int setup_tests(void) { #if OPENSSL_API_COMPAT < 0x10200000L @@ -205,5 +230,6 @@ int setup_tests(void) ADD_TEST(test_int64); ADD_TEST(test_uint64); ADD_TEST(test_invalid_template); + ADD_TEST(test_reuse_asn1_object); return 1; }