From: tb <tb@openbsd.org>
Date: Tue, 5 Sep 2023 14:59:00 +0000 (+0000)
Subject: Improve error handling in OBJ_add_object()
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=281c2125819493c8863e4ad9557474d0b8ef446f;p=openbsd

Improve error handling in OBJ_add_object()

There is no need for a helper function to obfuscate lh_ADDED_OBJ_new().
Just call the real thing directly.

Adding an object with a NID of NID_undef basically amounts to disabling
a built-in OID. It does so in an incoherent fashion and the caller can't
easily tell success from failure of the operation. Arguably the result is
a corrupted objects table.

Let's not allow adding such an object in an attempt at keeping things
slightly more coherent.

Issue noted and initial diff by schwarze while writing documentation

ok schwarze
---

diff --git a/lib/libcrypto/objects/obj_dat.c b/lib/libcrypto/objects/obj_dat.c
index 9c4c3179f38..6cfbf8f7733 100644
--- a/lib/libcrypto/objects/obj_dat.c
+++ b/lib/libcrypto/objects/obj_dat.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: obj_dat.c,v 1.60 2023/08/17 09:28:43 tb Exp $ */
+/* $OpenBSD: obj_dat.c,v 1.61 2023/09/05 14:59:00 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -208,15 +208,6 @@ added_obj_cmp(const ADDED_OBJ *ca, const ADDED_OBJ *cb)
 }
 static IMPLEMENT_LHASH_COMP_FN(added_obj, ADDED_OBJ)
 
-static int
-init_added(void)
-{
-	if (added != NULL)
-		return (1);
-	added = lh_ADDED_OBJ_new();
-	return (added != NULL);
-}
-
 static void
 cleanup1_doall(ADDED_OBJ *a)
 {
@@ -289,13 +280,16 @@ LCRYPTO_ALIAS(OBJ_new_nid);
 int
 OBJ_add_object(const ASN1_OBJECT *obj)
 {
-	ASN1_OBJECT *o;
+	ASN1_OBJECT *o = NULL;
 	ADDED_OBJ *ao[4] = {NULL, NULL, NULL, NULL}, *aop;
 	int i;
 
 	if (added == NULL)
-		if (!init_added())
-			return (0);
+		added = lh_ADDED_OBJ_new();
+	if (added == NULL)
+		goto err;
+	if (obj == NULL || obj->nid == NID_undef)
+		goto err;
 	if ((o = OBJ_dup(obj)) == NULL)
 		goto err;
 	if (!(ao[ADDED_NID] = malloc(sizeof(ADDED_OBJ))))