From d0cf9aeca512581235a63d9ed8e8a3c69039b9df Mon Sep 17 00:00:00 2001
From: schwarze <>
Date: Thu, 4 Jun 2020 21:21:03 +0000
Subject: 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@
---
 src/lib/libcrypto/asn1/x_attrib.c           |  7 +++++--
 src/lib/libcrypto/man/PKCS7_add_attribute.3 | 16 ++++------------
 2 files changed, 9 insertions(+), 14 deletions(-)

(limited to 'src/lib')

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
-- 
cgit v1.2.3-55-g6feb