[PATCH] Chunked filter: check if ctx is null
mdounin at mdounin.ru
Thu Jan 4 00:42:27 UTC 2018
On Wed, Jan 03, 2018 at 07:53:00PM +0100, Jan Prachař wrote:
> There exists a path which brings you to the body filter in the chunked
> filter module while the module ctx is null, which results in segfault.
> If during piping chunked response from upstream to downstream both
> upstream and downstream error happens, internal redirect to a named
> location is performed (accoring to the directive error_page) and
> module's contexts cleared. If you have a lua handler in that location,
> starts sending a body, because headers was already sent. A crash in the
> chunked filter module follows, because ctx is NULL.
> Maybe there is also a problem in the lua module and it should call
> header filters first. Also maybe nginx should not perform internal
> redirect, if part of the body was already sent.
> But better safe than sorry :) I found that the same checks are in body
> filters of other core modules too.
Trying to fix the chunked filter to tolerate such incorrect
behaviour looks like a bad idea. We can't reasonably assume all
filters are prepared to this. And even if we'll be able to modify
them all - if the connection remains open in such a situation, the
resulting mess at the protocol level will likely cause other
problems, including security ones. As such, the root cause should
be fixed instead.
To catch cases when a duplicate response is returned after the
header was already sent we have a dedicated check in the
ngx_http_send_header() function, see this commit for details:
Trying to bypass this check is a bad idea. The same applies to
conditionally sending headers based on the r->headers_sent flag,
as it will mean that the check will be bypassed. This is what the
lua module seems to be doing, and it should be fixed to avoid
The other part of the equation is how and why error_page is called
after the header as already sent. If you know a scenario where
error_page can be called with the header already sent, you may
want focus on reproducing and fixing this. Normally this is
expected to result in the "header already sent" alerts produced by
the check discussed above.
More information about the nginx-devel