From 63e1e004d5a60eea51f7c1fcc42d54aca4f605b9 Mon Sep 17 00:00:00 2001 From: schwarze <> Date: Tue, 6 Dec 2022 17:59:21 +0000 Subject: 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@ --- src/lib/libcrypto/bio/bio_lib.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) (limited to 'src/lib') diff --git a/src/lib/libcrypto/bio/bio_lib.c b/src/lib/libcrypto/bio/bio_lib.c index 2ffa0a765c..4c3d7ed5f5 100644 --- a/src/lib/libcrypto/bio/bio_lib.c +++ b/src/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); -- cgit v1.2.3-55-g6feb