On Mon, Nov 03, 2025 at 05:06:32PM +0100, Niels Dossche wrote:
> Hi
> 
> This patch fixes a memory leak when an error occurs in nref_nos when calling
> sk_ASN1_INTEGER_push in libressl. If the push operation fails, aint is never
> freed.
> 
> While here, also use the proper operation in sk_ASN1_INTEGER_pop_free,
> although they're synonyms so it doesn't really matter except for code style.
> 
> This issue was found via an experimental static analyzer I'm working on,
> and I manually read the code to verify whether this is a real bug or not.

Thanks. I committed these as they are correct in isolation.

However, there's a bigger problem here. nnums is not->noticeref and hangs
off the POLICYQUALINFO *qual. As such it is freed by POLICYQUALINFO_free().

If the error path in your patch is hit, we have a double free since
nref_nos() frees nnums and then POLICYQUALINFO_free(qual) will free it
a second time.

The patch below fixes this.

Index: x509/x509_cpols.c
===================================================================
RCS file: /cvs/src/lib/libcrypto/x509/x509_cpols.c,v
diff -u -p -r1.19 x509_cpols.c
--- x509/x509_cpols.c	3 Nov 2025 16:36:15 -0000	1.19
+++ x509/x509_cpols.c	3 Nov 2025 17:01:50 -0000
@@ -676,23 +676,18 @@ nref_nos(STACK_OF(ASN1_INTEGER) *nnums, 
 
 	for (i = 0; i < sk_CONF_VALUE_num(nos); i++) {
 		cnf = sk_CONF_VALUE_value(nos, i);
-		if (!(aint = s2i_ASN1_INTEGER(NULL, cnf->name))) {
+		if ((aint = s2i_ASN1_INTEGER(NULL, cnf->name)) == NULL) {
 			X509V3error(X509V3_R_INVALID_NUMBER);
-			goto err;
+			return 0;
 		}
-		if (!sk_ASN1_INTEGER_push(nnums, aint)) {
+		if (sk_ASN1_INTEGER_push(nnums, aint) <= 0) {
+			X509V3error(ERR_R_MALLOC_FAILURE);
 			ASN1_INTEGER_free(aint);
-			goto merr;
+			return 0;
 		}
 	}
-	return 1;
-
- merr:
-	X509V3error(ERR_R_MALLOC_FAILURE);
 
- err:
-	sk_ASN1_INTEGER_pop_free(nnums, ASN1_INTEGER_free);
-	return 0;
+	return 1;
 }
 
 static int