]> git.ipfire.org Git - thirdparty/openssl.git/commitdiff
Make SSL_get_stream_write_state() safe for concluded streams
authorAlexandr Nedvedicky <sashan@openssl.org>
Mon, 20 Apr 2026 20:35:16 +0000 (22:35 +0200)
committerEugene Syromiatnikov <esyr@openssl.org>
Tue, 28 Apr 2026 12:34:12 +0000 (14:34 +0200)
QUIC stack may panic when application calls SSL_get_stream_write_state()
on cocluded QUIC stream onject. The sequence of action which leads
to NULL pointer dereference is as follows:
  - application uses SSL_stream_conclude(ssl_stream, 0) to conclude
    the stream (let remote peer know no to expect more data)

  - application uses SSL_get_stream_write_state(ssl_stream)
    to query stream state.

If underlying sstream object is gone by the time when
SSL_get_stream_wtite_state() is called, then application
may see NULL pointer dereference. The underlying sstream
object is freed when FIN sent on beahalf of SSL_stream_conclude()
is ACKed by remote peer.

Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.foundation>
MergeDate: Tue Apr 28 12:35:41 2026
(Merged from https://github.com/openssl/openssl/pull/30913)

include/internal/quic_stream_map.h
ssl/quic/quic_impl.c
ssl/quic/quic_stream_map.c

index 925f516a097607c5f4f18ec02a11533055b021c1..36753d71965f0f00114c4338a1849df52d942c50 100644 (file)
@@ -299,7 +299,7 @@ struct quic_stream_st {
      *            STOP_SENDING.]
      *
      *            TODO(QUIC FUTURE): Implement the latter case (currently we
-                                     just always do STOP_SENDING).
+     *                               just always do STOP_SENDING).
      *
      *         and;
      *
@@ -315,6 +315,7 @@ struct quic_stream_st {
     unsigned int ready_for_gc : 1;
     /* Set to 1 if this is currently counted in the shutdown flush stream count. */
     unsigned int shutdown_flush : 1;
+    unsigned int have_final_size : 1;
 };
 
 #define QUIC_STREAM_INITIATOR_CLIENT 0
index 3d49857458e60dddf43e78847eaef6811537e26f..ab6da2f56604d231be3d290c91833aecc8f3020d 100644 (file)
@@ -4396,14 +4396,14 @@ static void quic_classify_stream(QUIC_CONNECTION *qc,
     uint64_t *app_error_code)
 {
     int local_init;
-    uint64_t final_size;
+    uint64_t scratch_pad; /* throw away value */
 
     local_init = (ossl_quic_stream_is_server_init(qs) == qc->as_server);
 
     if (app_error_code != NULL)
         *app_error_code = UINT64_MAX;
     else
-        app_error_code = &final_size; /* throw away value */
+        app_error_code = &scratch_pad;
 
     if (!ossl_quic_stream_is_bidi(qs) && local_init != is_write) {
         /*
@@ -4436,7 +4436,7 @@ static void quic_classify_stream(QUIC_CONNECTION *qc,
         *app_error_code = !is_write
             ? qs->peer_reset_stream_aec
             : qs->peer_stop_sending_aec;
-    } else if (is_write && ossl_quic_sstream_get_final_size(qs->sstream, &final_size)) {
+    } else if (is_write && qs->have_final_size) {
         /*
          * Stream has been finished. Stream reset takes precedence over this for
          * the write case as peer may not have received all data.
index 6f516e9cc89b8d097688b99ddd51710b9cd11cf3..b1605534523f12403adcd6ed151a153bb2a771aa 100644 (file)
@@ -446,6 +446,13 @@ int ossl_quic_stream_map_notify_totally_acked(QUIC_STREAM_MAP *qsm,
 
     case QUIC_SSTREAM_STATE_DATA_SENT:
         qs->send_state = QUIC_SSTREAM_STATE_DATA_RECVD;
+        /*
+         * Remember final size in case  SSL_get_stream_write_state()
+         * gets called.
+         */
+        qs->have_final_size = ossl_quic_sstream_get_final_size(qs->sstream,
+            NULL);
+
         /* We no longer need a QUIC_SSTREAM in this state. */
         ossl_quic_sstream_free(qs->sstream);
         qs->sstream = NULL;