From 9f4aeb928441e201f22864578423923ec1b137b1 Mon Sep 17 00:00:00 2001
From: tb <>
Date: Sun, 7 Dec 2025 09:27:02 +0000
Subject: Remove last internal use of ASN1_STRING_data()

PKCS5_pbe_set0_algor() is no longer public, but its parameters are
provided directly via public API, namely the wonderful PKCS8_encrypt()
and PKCS12_pack_p7encdata().

Muppetry abounds. To wit:

If saltlen < 0, the call to ASN1_STRING_set(pbe->salt, NULL, saltlen)
would error. Let's do that up front in a more obvious way. We don't
care about side-effects to pbe->salt since we free it on error anyway.

If saltlen == 0, we default it to PKCS5_PBE1_SALT_LEN. This is
particularly funky in case the caller passed in salt != NULL, in
which case we can only hope and pray this buffer is long enough.

If the caller passed a salt, copy it to pbe->salt via ASN1_STRING_set().

If there's no salt, allocate a buffer of the appropriate length, fill it
with random and transfer ownership to pbe->salt via ASN1_STRING_set0().
There's a change of behavior in that this will not be NUL-terminated
(why should it be?). If we wanted to preserve behavior, we'd just use
calloc(1, saltlen + 1) instead of the malloc().

The exit path is quite special, too, but I didn't want to change this
right now.

tweaks/ok kenjiro
---
 src/lib/libcrypto/asn1/p5_pbe.c | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

(limited to 'src/lib')

diff --git a/src/lib/libcrypto/asn1/p5_pbe.c b/src/lib/libcrypto/asn1/p5_pbe.c
index 668bf5d7c1..feccf8af58 100644
--- a/src/lib/libcrypto/asn1/p5_pbe.c
+++ b/src/lib/libcrypto/asn1/p5_pbe.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: p5_pbe.c,v 1.30 2025/05/24 02:57:14 tb Exp $ */
+/* $OpenBSD: p5_pbe.c,v 1.31 2025/12/07 09:27:02 tb Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 1999.
  */
@@ -129,7 +129,6 @@ PKCS5_pbe_set0_algor(X509_ALGOR *algor, int alg, int iter,
 {
 	PBEPARAM *pbe = NULL;
 	ASN1_STRING *pbe_str = NULL;
-	unsigned char *sstr;
 
 	if ((pbe = PBEPARAM_new()) == NULL) {
 		ASN1error(ERR_R_MALLOC_FAILURE);
@@ -141,17 +140,24 @@ PKCS5_pbe_set0_algor(X509_ALGOR *algor, int alg, int iter,
 		ASN1error(ERR_R_MALLOC_FAILURE);
 		goto err;
 	}
-	if (!saltlen)
-		saltlen = PKCS5_PBE1_SALT_LEN;
-	if (!ASN1_STRING_set(pbe->salt, NULL, saltlen)) {
-		ASN1error(ERR_R_MALLOC_FAILURE);
+	if (saltlen < 0)
 		goto err;
-	}
-	sstr = ASN1_STRING_data(pbe->salt);
-	if (salt)
-		memcpy(sstr, salt, saltlen);
-	else
+	if (saltlen == 0)
+		saltlen = PKCS5_PBE1_SALT_LEN;
+	if (salt != NULL) {
+		if (!ASN1_STRING_set(pbe->salt, salt, saltlen))
+			goto err;
+	} else {
+		unsigned char *sstr = NULL;
+
+		if ((sstr = malloc(saltlen)) == NULL) {
+			ASN1error(ERR_R_MALLOC_FAILURE);
+			goto err;
+		}
 		arc4random_buf(sstr, saltlen);
+		ASN1_STRING_set0(pbe->salt, sstr, saltlen);
+		sstr = NULL;
+	}
 
 	if (!ASN1_item_pack(pbe, &PBEPARAM_it, &pbe_str)) {
 		ASN1error(ERR_R_MALLOC_FAILURE);
@@ -165,9 +171,9 @@ PKCS5_pbe_set0_algor(X509_ALGOR *algor, int alg, int iter,
 		return 1;
 
  err:
-	if (pbe != NULL)
-		PBEPARAM_free(pbe);
+	PBEPARAM_free(pbe);
 	ASN1_STRING_free(pbe_str);
+
 	return 0;
 }
 
-- 
cgit v1.2.3-55-g6feb