def_load_bio(): Free |biosk| more carefully
authorRichard Levitte <levitte@openssl.org>
Mon, 23 Jul 2018 20:29:22 +0000 (22:29 +0200)
committerRichard Levitte <levitte@openssl.org>
Tue, 24 Jul 2018 07:50:56 +0000 (09:50 +0200)
If there's anything in the |biosk| stack, the first element is always
the input BIO.  It should never be freed in this function, so we must
take careful steps not to do so inadvertently when freeing the stack.

Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/6769)

crypto/conf/conf_def.c

index 676540cc6c35b8e6b77bd1570fd95ec39827850b..7f0d70ea695ecfffafcd81812e1f0e96ab06960c 100644 (file)
@@ -424,12 +424,26 @@ static int def_load_bio(CONF *conf, BIO *in, long *line)
     }
     BUF_MEM_free(buff);
     OPENSSL_free(section);
     }
     BUF_MEM_free(buff);
     OPENSSL_free(section);
-    sk_BIO_pop_free(biosk, BIO_vfree);
+    /*
+     * No need to pop, since we only get here if the stack is empty.
+     * If this causes a BIO leak, THE ISSUE IS SOMEWHERE ELSE!
+     */
+    sk_BIO_free(biosk);
     return 1;
  err:
     BUF_MEM_free(buff);
     OPENSSL_free(section);
     return 1;
  err:
     BUF_MEM_free(buff);
     OPENSSL_free(section);
-    sk_BIO_pop_free(biosk, BIO_vfree);
+    /*
+     * Since |in| is the first element of the stack and should NOT be freed
+     * here, we cannot use sk_BIO_pop_free().  Instead, we pop and free one
+     * BIO at a time, making sure that the last one popped isn't.
+     */
+    while (sk_BIO_num(biosk) > 0) {
+        BIO *popped = sk_BIO_pop(biosk);
+        BIO_vfree(in);
+        in = popped;
+    }
+    sk_BIO_free(biosk);
 #ifndef OPENSSL_NO_POSIX_IO
     OPENSSL_free(dirpath);
     if (dirctx != NULL)
 #ifndef OPENSSL_NO_POSIX_IO
     OPENSSL_free(dirpath);
     if (dirctx != NULL)