Make sure BIO_push(3) always preserves all invariants of the prev_bio

and next_bio fields of all BIO objects in all affected chains, no
matter what the arguments are.
In particular, if the second argument (the one to be appended) is
not at the beginning of its chain, properly detach the beginning
of its chain before appending.

We have weak indications that this bug might affect real-world code.
For example, in FreeRDP, file libfreerdp/crypto/tls.c, function
bio_rdp_tls_ctrl(), case BIO_C_SET_SSL, BIO_push(3) is definitely
called with a second argument that is *not* at the beginning of its
chain.  Admittedly, that code is hard to fathom, but it does appear
to result in a bogus prev_bio pointer without this patch.
The practical impact of this bug in this and other software remains
unknown; the consequences might possibly escalate up to use-after-free
issues if BIO_pop(3) is afterwards called on corrupted BIO objects.

OK tb@

diff --git a/lib/libcrypto/bio/bio_lib.c b/lib/libcrypto/bio/bio_lib.c
index 2ffa0a7..4c3d7ed 100644 (file)
--- a/lib/libcrypto/bio/bio_lib.c
+++ b/lib/libcrypto/bio/bio_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: bio_lib.c,v 1.40 2022/12/06 16:10:55 schwarze Exp $ */
+/* $OpenBSD: bio_lib.c,v 1.41 2022/12/06 17:59:21 schwarze Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -625,7 +625,11 @@ BIO_ctrl_wpending(BIO *bio)
 }
 
 
-/* put the 'bio' on the end of b's list of operators */
+/*
+ * Append "bio" to the end of the chain containing "b":
+ * Two chains "b -> lb" and "oldhead -> bio"
+ * become two chains "b -> lb -> bio" and "oldhead".
+ */
 BIO *
 BIO_push(BIO *b, BIO *bio)
 {
@@ -637,8 +641,11 @@ BIO_push(BIO *b, BIO *bio)
        while (lb->next_bio != NULL)
                lb = lb->next_bio;
        lb->next_bio = bio;
-       if (bio != NULL)
+       if (bio != NULL) {
+               if (bio->prev_bio != NULL)
+                       bio->prev_bio->next_bio = NULL;
                bio->prev_bio = lb;
+       }
        /* called to do internal processing */
        BIO_ctrl(b, BIO_CTRL_PUSH, 0, lb);
        return (b);