mail proxy leaves a stale upstream connection pointer after close

[xintenseapple]

Finding: mail proxy leaves a stale upstream connection pointer after close

Summary

In freenginx, the mail proxy closes s->proxy->upstream.connection on upstream-error and session-close paths but does not clear the pointer. The closed ngx_connection_t is returned to the global free list, leaving the mail session with a non-NULL alias to a reusable connection object.

Affected Area

• src/mail/ngx_mail_proxy_module.c
• Functions: ngx_mail_proxy_upstream_error(), ngx_mail_proxy_internal_server_error(), and ngx_mail_proxy_close_session()

Source Evidence

Several paths close the upstream connection without clearing the session pointer:

if (s->proxy->upstream.connection) {
    ngx_log_debug1(NGX_LOG_DEBUG_MAIL, s->connection->log, 0,
                   "close mail proxy connection: %d",
                   s->proxy->upstream.connection->fd);

    ngx_close_connection(s->proxy->upstream.connection);
}

The same pattern appears in the upstream-error, internal-server-error, and close-session paths. After ngx_close_connection(), the connection object can be reused for another accepted or upstream connection while the mail session still has a non-NULL pointer to it.

Expected Behavior

After closing the upstream connection, the mail proxy session should clear s->proxy->upstream.connection so later logic cannot observe a stale alias to a reusable connection object.

Impact

Potential stale pointer to a reused ngx_connection_t. Follow-on reads of the non-NULL upstream connection field can observe unrelated connection state after connection reuse. This needs timing validation to demonstrate a later dereference after reuse.

Reproduction Strategy

Force upstream proxy failures while keeping the client-side mail session alive, then churn connections so the closed upstream ngx_connection_t is reused. Add an assertion or poison check that s->proxy->upstream.connection is NULL after upstream close.

Suggested Fix

Set s->proxy->upstream.connection = NULL immediately after every close of that pointer.

[mdounin]

Thanks for reporting this. Similarly to #23, probably should be addressed with better handling of ngx_mail_send() with s->quit set.

[mdounin]

Looking into this in more detail, I tend to think that the only issue here might happen if an error is generated during the initial connection, when ngx_mail_proxy_block_read() is used as the client connection read handler. Both ngx_mail_proxy_upstream_error() and ngx_mail_proxy_internal_server_error() end up calling ngx_mail_send() with s->quit set, but fail to update the client connection read handler.

Similarly to #23, if ngx_mail_send() blocks, and a read event happens on the client connection, the existing read handler, ngx_mail_proxy_block_read() will be called. And in case of ngx_handle_read_event() failure (which is almost impossible in practice) it will call ngx_mail_proxy_close_session(), which will in turn try to close s->proxy->upstream.connection. In most cases, this is going to result in a "connection already closed" alert, though might also close an unrelated connection if the connection was already reused.

No other code paths seems to be affected. After the initial authentication, ngx_mail_proxy_handler() is used for all operations, and it only calls ngx_mail_proxy_close_session() in case of errors, which close the client connection immediately.

As such, similarly to #23, improved s->quit handling in ngx_mail_send() should be enough to resolve this issue. The patch is available here:

https://freenginx.org/pipermail/nginx-devel/2026-June/001186.html