From: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date: Fri, 27 Oct 2023 10:05:05 +0000 (+0200)
Subject: Fix error handling in OBJ_add_object
X-Git-Tag: openssl-3.1.7~12
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=934baa4c70a358cd5581d7893194688b90341337;p=thirdparty%2Fopenssl.git

Fix error handling in OBJ_add_object

This fixes the possible memory leak in OBJ_add_object
when a pre-existing object is replaced by a new one,
with identical NID, OID, and/or short/long name.
We do not try to delete any orphans, but only mark
them as type == -1, because the previously returned
pointers from OBJ_nid2obj/OBJ_nid2sn/OBJ_nid2ln
may be cached by applications and can thus not
be cleaned up before the application terminates.

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

(cherry picked from commit e91384d5b0547bf797e2b44976f142d146c4e650)
---

diff --git a/crypto/objects/obj_dat.c b/crypto/objects/obj_dat.c
index fd2b60b533c..f6b64bca391 100644
--- a/crypto/objects/obj_dat.c
+++ b/crypto/objects/obj_dat.c
@@ -273,7 +273,7 @@ int OBJ_new_nid(int num)
 static int ossl_obj_add_object(const ASN1_OBJECT *obj, int lock)
 {
     ASN1_OBJECT *o = NULL;
-    ADDED_OBJ *ao[4] = { NULL, NULL, NULL, NULL }, *aop;
+    ADDED_OBJ *ao[4] = { NULL, NULL, NULL, NULL }, *aop[4];
     int i;
 
     if ((o = OBJ_dup(obj)) == NULL)
@@ -306,9 +306,21 @@ static int ossl_obj_add_object(const ASN1_OBJECT *obj, int lock)
         if (ao[i] != NULL) {
             ao[i]->type = i;
             ao[i]->obj = o;
-            aop = lh_ADDED_OBJ_insert(added, ao[i]);
-            /* memory leak, but should not normally matter */
-            OPENSSL_free(aop);
+            aop[i] = lh_ADDED_OBJ_retrieve(added, ao[i]);
+            if (aop[i] != NULL)
+                aop[i]->type = -1;
+            (void)lh_ADDED_OBJ_insert(added, ao[i]);
+            if (lh_ADDED_OBJ_error(added)) {
+                if (aop[i] != NULL)
+                    aop[i]->type = i;
+                while (i-- > ADDED_DATA) {
+                    lh_ADDED_OBJ_delete(added, ao[i]);
+                    if (aop[i] != NULL)
+                        aop[i]->type = i;
+                }
+                ERR_raise(ERR_LIB_OBJ, ERR_R_CRYPTO_LIB);
+                goto err;
+            }
         }
     }
     o->flags &=
diff --git a/test/evp_extra_test.c b/test/evp_extra_test.c
index 3c88d12a3a5..06a3050300c 100644
--- a/test/evp_extra_test.c
+++ b/test/evp_extra_test.c
@@ -4720,6 +4720,7 @@ static int custom_md_cleanup(EVP_MD_CTX *ctx)
 
 static int test_custom_md_meth(void)
 {
+    ASN1_OBJECT *o = NULL;
     EVP_MD_CTX *mdctx = NULL;
     EVP_MD *tmp = NULL;
     char mess[] = "Test Message\n";
@@ -4765,8 +4766,21 @@ static int test_custom_md_meth(void)
             || !TEST_int_eq(custom_md_cleanup_called, 1))
         goto err;
 
+    if (!TEST_int_eq(OBJ_create("1.3.6.1.4.1.16604.998866.1",
+                                "custom-md", "custom-md"), NID_undef)
+            || !TEST_int_eq(ERR_GET_LIB(ERR_peek_error()), ERR_LIB_OBJ)
+            || !TEST_int_eq(ERR_GET_REASON(ERR_get_error()), OBJ_R_OID_EXISTS))
+        goto err;
+
+    o = ASN1_OBJECT_create(nid, (unsigned char *)
+                                "\53\6\1\4\1\201\201\134\274\373\122\1", 12,
+                                "custom-md", "custom-md");
+    if (!TEST_int_eq(OBJ_add_object(o), nid))
+        goto err;
+
     testresult = 1;
  err:
+    ASN1_OBJECT_free(o);
     EVP_MD_CTX_free(mdctx);
     EVP_MD_meth_free(tmp);
     return testresult;