Improve the poorly designed BIO_set_next(3) API to always preserve all

invariants of the prev_bio and next_bio fields of all BIO objects
in all involved chains, no matter which arguments this function is
called with.

Both real-world uses of this function (in libssl and freerdp) have
been audited to make sure this makes nothing worse.  We believe libssl
behaves correctly before and after the patch (mostly because the second
argument is NULL there), and we believe the code in freerdp behaves
incorrectly before and after the patch, leaving a prev_bio pointer in
place that is becoming bogus, only in a different object before and
after the patch.  But after the patch, that bogus pointer is due to a
separate bug in BIO_push(3), which we are planning to fix afterwards.

Joint work with and OK tb@.

diff --git a/lib/libcrypto/bio/bio_lib.c b/lib/libcrypto/bio/bio_lib.c
index c09ad8f..2ffa0a7 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.39 2022/12/02 19:44:04 tb Exp $ */
+/* $OpenBSD: bio_lib.c,v 1.40 2022/12/06 16:10:55 schwarze Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -727,10 +727,25 @@ BIO_next(BIO *b)
        return b->next_bio;
 }
 
+/*
+ * Two chains "bio -> oldtail" and "oldhead -> next" become
+ * three chains "oldtail", "bio -> next", and "oldhead".
+ */
 void
-BIO_set_next(BIO *b, BIO *next)
+BIO_set_next(BIO *bio, BIO *next)
 {
-       b->next_bio = next;
+       /* Cut off the tail of the chain containing bio after bio. */
+       if (bio->next_bio != NULL)
+               bio->next_bio->prev_bio = NULL;
+
+       /* Cut off the head of the chain containing next before next. */
+       if (next != NULL && next->prev_bio != NULL)
+               next->prev_bio->next_bio = NULL;
+
+       /* Append the chain starting at next to the chain ending at bio. */
+       bio->next_bio = next;
+       if (next != NULL)
+               next->prev_bio = bio;
 }
 
 void