When X509_ATTRIBUTE_create() receives an invalid NID (e.g., -1), return

failure rather than silently constructing a broken X509_ATTRIBUTE object
that might cause NULL pointer accesses later on.  This matters because
X509_ATTRIBUTE_create() is used by documented API functions like
PKCS7_add_attribute(3) and the NID comes straight from the user.

This fixes a bug found while working on documentation.

OK tb@  and  "thanks" bluhm@

2 files changed, 9 insertions, 14 deletions

diff --git a/src/lib/libcrypto/asn1/x_attrib.c b/src/lib/libcrypto/asn1/x_attrib.c
index bb74a1b6c7..04816eab77 100644
--- a/src/lib/libcrypto/asn1/x_attrib.c
+++ b/src/lib/libcrypto/asn1/x_attrib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: x_attrib.c,v 1.13 2015/02/14 14:56:45 jsing Exp $ */
+/* $OpenBSD: x_attrib.c,v 1.14 2020/06/04 21:21:03 schwarze Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -174,10 +174,13 @@ X509_ATTRIBUTE_create(int nid, int atrtype, void *value)
 {
         X509_ATTRIBUTE *ret = NULL;
         ASN1_TYPE *val = NULL;
+        ASN1_OBJECT *oid;
 
+        if ((oid = OBJ_nid2obj(nid)) == NULL)
+                return (NULL);
         if ((ret = X509_ATTRIBUTE_new()) == NULL)
                 return (NULL);
-        ret->object = OBJ_nid2obj(nid);
+        ret->object = oid;
         ret->single = 0;
         if ((ret->value.set = sk_ASN1_TYPE_new_null()) == NULL)
                 goto err;
diff --git a/src/lib/libcrypto/man/PKCS7_add_attribute.3 b/src/lib/libcrypto/man/PKCS7_add_attribute.3
index 09c36a4d5d..081703f0f3 100644
--- a/src/lib/libcrypto/man/PKCS7_add_attribute.3
+++ b/src/lib/libcrypto/man/PKCS7_add_attribute.3
@@ -1,4 +1,4 @@
-.\" $OpenBSD: PKCS7_add_attribute.3,v 1.1 2020/06/04 10:24:27 schwarze Exp $
+.\" $OpenBSD: PKCS7_add_attribute.3,v 1.2 2020/06/04 21:21:03 schwarze Exp $
 .\"
 .\" Copyright (c) 2020 Ingo Schwarze <schwarze@openbsd.org>
 .\"
@@ -123,7 +123,9 @@ exist.
 and
 .Fn PKCS7_add_signed_attribute
 return 1 on success or 0 on failure.
-The most common reason for failure is lack of memory.
+The most common reasons for failure are an invalid
+.Fa nid
+argument or lack of memory.
 .Pp
 .Fn PKCS7_get_attribute
 and
@@ -153,16 +155,6 @@ These functions first appeared in OpenSSL 0.9.1
 and have been available since
 .Ox 2.6 .
 .Sh BUGS
-Adding an attribute with an invalid
-.Fa nid
-ought to fail, but it actually succeeds
-setting the type of the new attribute to
-.Dv NULL .
-Subsequent attempts to retrieve attributes
-may cause the program to crash due to
-.Dv NULL
-pointer access.
-.Pp
 A function to remove individual attributes from these lists
 does not appear to exist.
 A program desiring to do that might have to manually iterate the fields