From 46665642565a159e2ac7f0f7095bc41fe08d50ef Mon Sep 17 00:00:00 2001 From: schwarze Date: Tue, 6 Dec 2022 17:59:21 +0000 Subject: [PATCH] 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@ --- lib/libcrypto/bio/bio_lib.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/libcrypto/bio/bio_lib.c b/lib/libcrypto/bio/bio_lib.c index 2ffa0a765c4..4c3d7ed5f57 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.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); -- 2.20.1