From bbfffbcaf38dff61fe7a1fcbfa6af9a818e1e188 Mon Sep 17 00:00:00 2001 From: Neil Horman Date: Thu, 7 Nov 2024 20:01:48 -0500 Subject: [PATCH] Fix SSL_stream_reset for stream objects which have FIN bit set When calling SSL_stream_reset on a QUIC stream object that has received all data that is expected to be sent (i.e. when the sender has sent a STREAM frame with the FIN bit set), we encounter the following segfault: Program received signal SIGSEGV, Segmentation fault. 0x00007ffff7f0bd28 in ossl_quic_sstream_get_final_size (qss=0x0, final_size=0x0) at ssl/quic/quic_sstream.c:273 273 if (!qss->have_final_size) (gdb) bt 0) 0x00007ffff7f0bd28 in ossl_quic_sstream_get_final_size (qss=0x0, final_size=0x0) at ssl/quic/quic_sstream.c:273 1) 0x00007ffff7ef65bf in quic_validate_for_write (xso=0x5555555efcb0, err=0x7fffffffd5e0) at ssl/quic/quic_impl.c:2513 2) 0x00007ffff7ef8ae3 in ossl_quic_stream_reset (ssl=0x5555555efcb0, args=0x0, args_len=0) at ssl/quic/quic_impl.c:3657 3) 0x00007ffff7ebdaa6 in SSL_stream_reset (s=0x5555555efcb0, args=0x0, args_len=0) at ssl/ssl_lib.c:7635 4) 0x0000555555557527 in build_request_set ( req_list=0x55555555ebd0 "neil1.txt neil2.txt neil3.txt neil4.txt neil5.txt neil6.txt neil7.txt neil8.txt neil9.txt neil10.txt neil11.txt neil12.txt neil13.txt neil14.txt neil15.txt neil16.txt neil17.txt neil18.txt neil19.txt "..., ssl=0x5555555b6f80) at demos/guide/quic-hq-interop.c:545 5) 0x00005555555587b2 in main (argc=4, argv=0x7fffffffe568) at demos/guide/quic-hq-interop.c:941 This occurs because: 1) When the stream FIN bit is set, the quic stack frees the underlying stream structures immediately within the QUIC stack and 2) when SSL_stream_reset is called, the call stack indicates we call quic_validate_for_write, which attempts to access the xso->stream->sstream QUIC_SSTREAM object, which was already freed in (1) The fix I think is pretty straightforward. On receipt of a STREAM frame with a FIN bit set, the QUIC stack sets the QUIC_STREAM object state to QUIC_SSTREAM_STATE_DATA_RECVD, which means we can use that state to simply assert that the stream is valid for write, which allows it to be reset properly. Fixes #25410 Reviewed-by: Matt Caswell Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/25910) --- ssl/quic/quic_impl.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ssl/quic/quic_impl.c b/ssl/quic/quic_impl.c index 763540a941..c603ee9b1f 100644 --- a/ssl/quic/quic_impl.c +++ b/ssl/quic/quic_impl.c @@ -2509,14 +2509,16 @@ static int quic_validate_for_write(QUIC_XSO *xso, int *err) /* FALLTHROUGH */ case QUIC_SSTREAM_STATE_SEND: case QUIC_SSTREAM_STATE_DATA_SENT: - case QUIC_SSTREAM_STATE_DATA_RECVD: if (ossl_quic_sstream_get_final_size(xso->stream->sstream, NULL)) { *err = SSL_R_STREAM_FINISHED; return 0; } - return 1; + case QUIC_SSTREAM_STATE_DATA_RECVD: + *err = SSL_R_STREAM_FINISHED; + return 0; + case QUIC_SSTREAM_STATE_RESET_SENT: case QUIC_SSTREAM_STATE_RESET_RECVD: *err = SSL_R_STREAM_RESET;