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)
This commit is contained in:
Bernd Edlinger 2023-10-27 12:05:05 +02:00 committed by Tomas Mraz
parent 223e0020e4
commit e91384d5b0
2 changed files with 30 additions and 4 deletions

View File

@ -263,7 +263,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)
@ -294,9 +294,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 &=

View File

@ -4944,6 +4944,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";
@ -4989,8 +4990,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;