Index | Thread | Search

From:
Theo Buehler <tb@theobuehler.org>
Subject:
Re: [PATCH] libressl: fix memory leak in save_index on error
To:
Niels Dossche <dossche.niels@gmail.com>
Cc:
tech@openbsd.org, jsing@openbsd.org, beck@openbsd.org
Date:
Mon, 17 Mar 2025 17:09:51 +0100

Download raw body.

Thread
On Mon, Mar 17, 2025 at 04:49:49PM +0100, Niels Dossche wrote:
> Hi
> 
> This patch fixes a memory leak when an error occurs in save_index in libressl.

On top of this let's address a few more code quality issues in here.

- Error check both BIO_new() calls.
- j isn't used, so drop it.
- Don't succeed if writing the unique_subject fails
- might as well turn the function into single exit.

Index: apps.c
===================================================================
RCS file: /cvs/src/usr.bin/openssl/apps.c,v
diff -u -p -r1.71 apps.c
--- apps.c	17 Mar 2025 15:55:09 -0000	1.71
+++ apps.c	17 Mar 2025 16:08:02 -0000
@@ -1377,10 +1377,10 @@ int
 save_index(const char *file, const char *suffix, CA_DB *db)
 {
 	char attrpath[PATH_MAX], dbfile[PATH_MAX];
-	BIO *out = BIO_new(BIO_s_file());
-	int j;
+	BIO *out;
+	int ret = 0;
 
-	if (out == NULL) {
+	if ((out = BIO_new(BIO_s_file())) == NULL) {
 		ERR_print_errors(bio_err);
 		goto err;
 	}
@@ -1400,28 +1400,31 @@ save_index(const char *file, const char 
 		BIO_printf(bio_err, "unable to open '%s'\n", dbfile);
 		goto err;
 	}
-	j = TXT_DB_write(out, db->db);
-	if (j <= 0)
+
+	if (TXT_DB_write(out, db->db) <= 0)
 		goto err;
 
 	BIO_free(out);
-
-	out = BIO_new(BIO_s_file());
+	if ((out = BIO_new(BIO_s_file())) == NULL) {
+		ERR_print_errors(bio_err);
+		goto err;
+	}
 
 	if (BIO_write_filename(out, attrpath) <= 0) {
 		perror(attrpath);
 		BIO_printf(bio_err, "unable to open '%s'\n", attrpath);
 		goto err;
 	}
-	BIO_printf(out, "unique_subject = %s\n",
-	    db->attributes.unique_subject ? "yes" : "no");
-	BIO_free(out);
+	if ((BIO_printf(out, "unique_subject = %s\n",
+	    db->attributes.unique_subject ? "yes" : "no")) <= 0)
+		goto err;
 
-	return 1;
+	ret = 1;
 
  err:
 	BIO_free(out);
-	return 0;
+
+	return ret;
 }
 
 int