From: Theo Buehler Subject: Re: [PATCH] libressl: Fix memory leak in nref_nos on error To: Niels Dossche Cc: tech@openbsd.org, jsing@openbsd.org, kenjiro@openbsd.org, beck@openbsd.org Date: Mon, 3 Nov 2025 18:13:33 +0100 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