Fix BIO_pop for SSL BIOs

The BIO_pop implementation assumes that the rbio still equals the next BIO
in the chain. While this would normally be the case, it is possible that it
could have been changed directly by the application. It also does not
properly cater for the scenario where the buffering BIO is still in place
for the write BIO.

Most of the existing BIO_pop code for SSL BIOs can be replaced by a single
call to SSL_set_bio(). This is equivalent to the existing code but
additionally handles the scenario where the rbio has been changed or the
buffering BIO is still in place.

Reviewed-by: Rich Salz <rsalz@openssl.org>

diff --git a/ssl/bio_ssl.c b/ssl/bio_ssl.c
index 5212a7b239e445284cdafd39069c542602503723..3dd09cf52de2e36114772e70daf0f15f70031cf5 100644 (file)
--- a/ssl/bio_ssl.c
+++ b/ssl/bio_ssl.c
@@ -338,16 +338,8 @@ static long ssl_ctrl(BIO *b, int cmd, long num, void *ptr)
     case BIO_CTRL_POP:
         /* Only detach if we are the BIO explicitly being popped */
         if (b == ptr) {
     case BIO_CTRL_POP:
         /* Only detach if we are the BIO explicitly being popped */
         if (b == ptr) {

-            /*
-             * Shouldn't happen in practice because the rbio and wbio are the
-             * same when pushed.
-             */
-            if (ssl->rbio != ssl->wbio)
-                BIO_free_all(ssl->wbio);
-            if (next != NULL)
-                BIO_free(next);
-            ssl->wbio = NULL;
-            ssl->rbio = NULL;
+            /* This will clear the reference we obtained during push */
+            SSL_set_bio(ssl, NULL, NULL);

         }
         break;
     case BIO_C_DO_STATE_MACHINE:
         }
         break;
     case BIO_C_DO_STATE_MACHINE: