bugfix: prevent use-after-free crash in ngx_http_lua_pipe by ensuring connections are closed before pool destruction in quic connection close path.

Author: oowl

src/ngx_http_lua_pipe.c

@@ -1200,6 +1200,40 @@ ngx_http_lua_ffi_pipe_proc_destroy(ngx_http_lua_ffi_pipe_proc_t *proc)
     }
 
     ngx_http_lua_pipe_proc_finalize(proc);
+
+    /*
+     * pipe_proc_finalize -> ngx_http_lua_pipe_close_helper may leave pipe
+     * connections open with active timers/posted events when there are
+     * pending I/O operations (handler != dummy_handler).  Close them now
+     * before destroying the pool.
+     *
+     * Without this, when pool cleanup LIFO ordering causes pipe_proc_destroy
+     * to run before request_cleanup_handler (e.g. QUIC connection close path:
+     * ngx_quic_close_streams -> ngx_http_free_request -> ngx_destroy_pool),
+     * request_cleanup sees proc->pipe == NULL and returns early, leaving the
+     * timer live.  The timer then fires after the request pool is freed,
+     * accessing a dangling wait_co_ctx pointer -> SIGSEGV.
+     *
+     * ngx_close_connection handles everything: timers (ngx_del_timer),
+     * posted events (ngx_delete_posted_event), epoll removal, fd close,
+     * and connection recycling.
+     */
+
+    if (pipe->stdout_ctx && pipe->stdout_ctx->c) {
+        ngx_close_connection(pipe->stdout_ctx->c);
+        pipe->stdout_ctx->c = NULL;
+    }
+
+    if (pipe->stderr_ctx && pipe->stderr_ctx->c) {
+        ngx_close_connection(pipe->stderr_ctx->c);
+        pipe->stderr_ctx->c = NULL;
+    }
+
+    if (pipe->stdin_ctx && pipe->stdin_ctx->c) {
+        ngx_close_connection(pipe->stdin_ctx->c);
+        pipe->stdin_ctx->c = NULL;
+    }
+
     ngx_destroy_pool(pipe->pool);
     proc->pipe = NULL;
 }
@@ -2495,13 +2529,20 @@ ngx_http_lua_pipe_proc_read_stdout_cleanup(void *data)
                    "lua pipe proc read stdout cleanup");
 
     proc = wait_co_ctx->data;
+
+    wait_co_ctx->cleanup = NULL;
+
+    if (proc->pipe == NULL) {
+        /* pipe_proc_destroy already ran (LIFO pool cleanup) and cancelled
+         * timers/connections; nothing left to clean up here. */
+        return;
+    }
+
     c = proc->pipe->stdout_ctx->c;
     if (c) {
         rev = c->read;
         ngx_http_lua_pipe_clear_event(rev);
     }
-
-    wait_co_ctx->cleanup = NULL;
 }
 
 
@@ -2517,13 +2558,18 @@ ngx_http_lua_pipe_proc_read_stderr_cleanup(void *data)
                    "lua pipe proc read stderr cleanup");
 
     proc = wait_co_ctx->data;
+
+    wait_co_ctx->cleanup = NULL;
+
+    if (proc->pipe == NULL) {
+        return;
+    }
+
     c = proc->pipe->stderr_ctx->c;
     if (c) {
         rev = c->read;
         ngx_http_lua_pipe_clear_event(rev);
     }
-
-    wait_co_ctx->cleanup = NULL;
 }
 
 
@@ -2539,13 +2585,18 @@ ngx_http_lua_pipe_proc_write_cleanup(void *data)
                    "lua pipe proc write cleanup");
 
     proc = wait_co_ctx->data;
+
+    wait_co_ctx->cleanup = NULL;
+
+    if (proc->pipe == NULL) {
+        return;
+    }
+
     c = proc->pipe->stdin_ctx->c;
     if (c) {
         wev = c->write;
         ngx_http_lua_pipe_clear_event(wev);
     }
-
-    wait_co_ctx->cleanup = NULL;
 }
 
 
@@ -2561,13 +2612,18 @@ ngx_http_lua_pipe_proc_wait_cleanup(void *data)
                    "lua pipe proc wait cleanup");
 
     proc = wait_co_ctx->data;
+
+    wait_co_ctx->cleanup = NULL;
+
+    if (proc->pipe == NULL) {
+        return;
+    }
+
     node = proc->pipe->node;
     pipe_node = (ngx_http_lua_pipe_node_t *) &node->color;
     pipe_node->wait_co_ctx = NULL;
 
     ngx_http_lua_pipe_clear_event(&wait_co_ctx->sleep);
-
-    wait_co_ctx->cleanup = NULL;
 }
 
 

t/191-pipe-proc-quic-close-crash.t

@@ -0,0 +1,101 @@
+# vim:set ft= ts=4 sw=4 et fdm=marker:
+#
+# Regression test for use-after-free crash in ngx_http_lua_pipe_resume_read_stdout_handler.
+#
+# Root cause: pool cleanup runs in LIFO order.  pipe_proc_destroy (registered
+# when the pipe is spawned, i.e. *later*) therefore runs *before*
+# request_cleanup_handler (registered at Lua handler init time).
+# pipe_proc_destroy calls pipe_proc_finalize → ngx_http_lua_pipe_close_helper,
+# which, when a read is in progress, posts the event rather than calling
+# ngx_close_connection, leaving the read-timeout timer live.  It then sets
+# proc->pipe = NULL.  When request_cleanup_handler runs next it sees
+# proc->pipe == NULL and returns early, so the timer is never cancelled.
+# After ngx_destroy_pool(r->pool) frees wait_co_ctx, the timer fires and
+# dereferences the dangling pointer → SIGSEGV.
+#
+# Trigger path (QUIC-specific):
+#   QUIC connection close
+#   → ngx_quic_close_streams → sc->read->handler (no-op without lua_check_client_abort)
+#   → ngx_http_free_request → ngx_destroy_pool(r->pool)
+#   → LIFO pool cleanups: pipe_proc_destroy first, request_cleanup_handler second
+#   → timer remains live; pool freed; timer fires → SIGSEGV
+#
+# Fix (ngx_http_lua_ffi_pipe_proc_destroy): after pipe_proc_finalize, call
+# ngx_close_connection on any pipe ctx whose connection is still open.
+# ngx_close_connection removes both the read-timeout timer (ngx_del_timer)
+# and any already-posted event (ngx_delete_posted_event), preventing the UAF.
+#
+# Timing:
+#   pipe stdout read timeout : 1 s  (timer armed 1 s after request lands)
+#   curl --max-time           : 0.5 s  (QUIC connection closed while timer live)
+#   --- wait                  : 2 s  (covers remaining ~0.5 s + safety margin)
+
+
+BEGIN {
+    if (!$ENV{TEST_NGINX_USE_HTTP3}) {
+        $SkipReason = "TEST_NGINX_USE_HTTP3 is not set";
+    } else {
+        my $nginx = $ENV{TEST_NGINX_BINARY} || 'nginx';
+        my $v     = eval { `$nginx -V 2>&1` } // '';
+
+        if ($v !~ /--with-http_v3_module/) {
+            $SkipReason = "requires nginx built with --with-http_v3_module";
+        }
+    }
+}
+
+use Test::Nginx::Socket::Lua $SkipReason ? (skip_all => $SkipReason) : ();
+
+master_on();   # master process needed to detect/survive worker crash
+workers(1);
+
+repeat_each(1);
+
+# 1 subtest per block: the --- no_error_log [alert] check.
+# (--- ignore_response suppresses the default status-code subtest.)
+plan tests => repeat_each() * blocks();
+
+log_level('info');
+no_long_string();
+
+add_block_preprocessor(sub {
+    my $block = shift;
+
+    my $http_config = $block->http_config || '';
+    $http_config .= <<'_EOC_';
+    lua_package_path "../lua-resty-core/lib/?.lua;../lua-resty-lrucache/lib/?.lua;;";
+
+    init_by_lua_block {
+        require "resty.core"
+    }
+_EOC_
+    $block->set_value("http_config", $http_config);
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: pipe read timer must not fire after pool is freed on QUIC connection close
+--- config
+    location = /t {
+        content_by_lua_block {
+            -- Spawn a long-lived child; stdout will never produce output.
+            -- set_timeouts(write_timeout, stdout_timeout, stderr_timeout, wait_timeout)
+            local proc = require("ngx.pipe").spawn({"sleep", "100"})
+            proc:set_timeouts(nil, 1000)  -- 1 s stdout read timeout
+
+            -- This call yields and arms a 1 s timer.
+            -- The test client closes the QUIC connection before the timer fires,
+            -- triggering the LIFO pool-cleanup use-after-free (without the fix).
+            proc:stdout_read_line()
+        }
+    }
+--- request
+GET /t
+--- timeout: 0.5
+--- wait: 2
+--- ignore_response
+--- curl_error eval: qr/\(28\)/
+--- no_error_log
+[alert]