bugfix: prevent use-after-free crash in ngx_http_lua_pipe by ensuring…

[oowl]

… connections are closed before pool destruction in quic connection close path.

I hereby granted the copyright of the changes in this pull request
to the authors of this lua-nginx-module project.

fix crash

gdb-peda$ bt
#0  0x000055da425e387f in ngx_http_lua_pipe_proc_read_stdout_cleanup (data=0x55da709beab8)
    at /home/owl/work/openresty/lua-nginx-module/src/ngx_http_lua_pipe.c:2498
#1  0x000055da425adfef in ngx_http_lua_cleanup_pending_operation (coctx=0x55da709beab8)
    at /home/owl/work/openresty/lua-nginx-module/src/ngx_http_lua_util.h:483
#2  ngx_http_lua_finalize_threads (r=r@entry=0x55da70a1f920, ctx=ctx@entry=0x55da709bea90, L=0x7fce77bd5380)
    at /home/owl/work/openresty/lua-nginx-module/src/ngx_http_lua_util.c:3381
#3  0x000055da425ae71c in ngx_http_lua_request_cleanup (ctx=0x55da709bea90, forcible=forcible@entry=0x0)
    at /home/owl/work/openresty/lua-nginx-module/src/ngx_http_lua_util.c:1106
#4  0x000055da425ae732 in ngx_http_lua_request_cleanup_handler (data=<optimized out>)
    at /home/owl/work/openresty/lua-nginx-module/src/ngx_http_lua_util.c:1064
#5  0x000055da4248aab9 in ngx_destroy_pool (pool=0x55da70a1d050) at src/core/ngx_palloc.c:48
#6  0x000055da424ef248 in ngx_http_free_request (r=r@entry=0x55da70a1f920, rc=rc@entry=0x0) at src/http/ngx_http_request.c:3963
#7  0x000055da424ef63e in ngx_http_close_request (r=r@entry=0x55da70a1f920, rc=rc@entry=0x0) at src/http/ngx_http_request.c:3867
#8  0x000055da424ef8aa in ngx_http_terminate_handler (r=0x55da70a1f920) at src/http/ngx_http_request.c:2886
#9  0x000055da424eefe2 in ngx_http_run_posted_requests (c=c@entry=0x7fce741dd9c0) at src/http/ngx_http_request.c:2608
#10 0x000055da424ef86d in ngx_http_request_handler (ev=0x55da709ea330) at src/http/ngx_http_request.c:2560
#11 0x000055da424ac8af in ngx_event_process_posted (cycle=0x55da7091b470, posted=posted@entry=0x7ffc1531a470)
    at src/event/ngx_event_posted.c:35
#12 0x000055da424d9c34 in ngx_quic_close_streams (c=c@entry=0x7fce741dd3f0, qc=qc@entry=0x55da7092a950)
    at src/event/quic/ngx_event_quic_streams.c:231
#13 0x000055da424cab1b in ngx_quic_close_connection (c=0x7fce741dd3f0, rc=rc@entry=0x0) at src/event/quic/ngx_event_quic.c:558
#14 0x000055da424cb1e2 in ngx_quic_close_handler (ev=0x55da7092b018) at src/event/quic/ngx_event_quic.c:668
#15 0x000055da424ac8af in ngx_event_process_posted (cycle=cycle@entry=0x55da7091b470, posted=0x55da426c99a0 <ngx_posted_events>)
    at src/event/ngx_event_posted.c:35
#16 0x000055da424ac1fb in ngx_process_events_and_timers (cycle=cycle@entry=0x55da7091b470) at src/event/ngx_event.c:273
#17 0x000055da424b6198 in ngx_worker_process_cycle (cycle=cycle@entry=0x55da7091b470, data=data@entry=0x0)
    at src/os/unix/ngx_process_cycle.c:793
#18 0x000055da424b450b in ngx_spawn_process (cycle=cycle@entry=0x55da7091b470, 
    proc=proc@entry=0x55da424b6080 <ngx_worker_process_cycle>, data=data@entry=0x0, 
    name=name@entry=0x55da4262de1f "worker process", respawn=respawn@entry=0xfffffffffffffffd) at src/os/unix/ngx_process.c:199
#19 0x000055da424b5118 in ngx_start_worker_processes (cycle=cycle@entry=0x55da7091b470, n=0x1, type=type@entry=0xfffffffffffffffd)
    at src/os/unix/ngx_process_cycle.c:382
#20 0x000055da424b6beb in ngx_master_process_cycle (cycle=cycle@entry=0x55da7091b470) at src/os/unix/ngx_process_cycle.c:135
#21 0x000055da4248919a in main (argc=<optimized out>, argv=<optimized out>) at src/core/nginx.c:387
#22 0x00007fce770366c1 in ?? () from /usr/lib/libc.so.6
#23 0x00007fce770367f9 in __libc_start_main () from /usr/lib/libc.so.6
#24 0x000055da42487765 in _start ()

POC

# 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)
#
# NOTE on Test::Nginx::Util internals:
#   ssl_certificate is injected into the server block ONLY when
#   ($UseHttp3 && !defined $block->http3), i.e. only in global HTTP3 mode.
#   Using a per-block "--- http3" directive without TEST_NGINX_USE_HTTP3=1
#   skips the ssl_certificate injection and nginx refuses to start.
#   Therefore this test enables global HTTP3 mode in a BEGIN block so that
#   the module reads the env vars at load time.

BEGIN {
    require File::Basename;
    require Cwd;

    # Default cert paths (generated by `util/build-and-test.sh --http3`).
    my $t_dir = File::Basename::dirname(Cwd::abs_path(__FILE__));
    $ENV{TEST_NGINX_HTTP3_CRT} = "$t_dir/cert/http3/http3.crt"
        unless defined $ENV{TEST_NGINX_HTTP3_CRT};
    $ENV{TEST_NGINX_HTTP3_KEY} = "$t_dir/cert/http3/http3.key"
        unless defined $ENV{TEST_NGINX_HTTP3_KEY};

    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";

    } elsif (!-f $ENV{TEST_NGINX_HTTP3_CRT} || !-f $ENV{TEST_NGINX_HTTP3_KEY}) {
        $SkipReason =
            "requires TEST_NGINX_HTTP3_CRT / TEST_NGINX_HTTP3_KEY cert files "
            . "(run util/build-and-test.sh --http3 once to generate them)";

    } else {
        # Enable global HTTP3 mode so Test::Nginx::Util injects ssl_certificate
        # into the server block and makes curl use --http3-only for all tests.
        $ENV{TEST_NGINX_USE_HTTP3} = 1 unless defined $ENV{TEST_NGINX_USE_HTTP3};
    }
}

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]

[Copilot]

Pull request overview

Adds a regression test and hardens ngx.pipe process cleanup to prevent a QUIC-triggered use-after-free crash when a request pool is destroyed while a pipe read timeout/event is still pending.

Changes:

• Ensure ngx_http_lua_ffi_pipe_proc_destroy force-closes any still-open pipe connections after finalization to cancel timers and remove posted events before destroying the pipe pool.
• Make pipe read/write/wait coroutine cleanup handlers tolerate LIFO cleanup ordering by returning early when proc->pipe is already NULL.
• Add an HTTP/3 (QUIC) regression test that reproduces the crash via client timeout during a yielded stdout_read_line().

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/ngx_http_lua_pipe.c	Closes lingering pipe connections during proc destroy and adds cleanup guards to avoid UAF in QUIC request teardown.
t/191-pipe-proc-quic-close-crash.t	New HTTP/3 regression test that asserts no worker crash/alert logs on QUIC connection close during an in-flight pipe read timeout.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.