From 85aebfcc6eedceaed34012a8b2c27c43ef402f95 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Mon, 23 Jul 2018 22:29:22 +0200 Subject: [PATCH] def_load_bio(): Free |biosk| more carefully 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 (Merged from https://github.com/openssl/openssl/pull/6769) --- crypto/conf/conf_def.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/crypto/conf/conf_def.c b/crypto/conf/conf_def.c index 676540cc6c..7f0d70ea69 100644 --- a/crypto/conf/conf_def.c +++ b/crypto/conf/conf_def.c @@ -424,12 +424,26 @@ static int def_load_bio(CONF *conf, BIO *in, long *line) } 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); - 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) -- 2.25.1