From 0dc4ae8263195d1db55bebe0394f9fa5cb5cdfc2 Mon Sep 17 00:00:00 2001 From: schwarze Date: Tue, 6 Dec 2022 16:10:55 +0000 Subject: [PATCH] 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@. --- lib/libcrypto/bio/bio_lib.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/lib/libcrypto/bio/bio_lib.c b/lib/libcrypto/bio/bio_lib.c index c09ad8fe8e2..2ffa0a765c4 100644 --- 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 -- 2.20.1