[PATCH] Chunked filter: check if ctx is null

[Maxim Dounin]

Hello!

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,
> it
> 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:

http://hg.nginx.org/nginx/rev/03ff14058272

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 
doing this.

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.

-- 
Maxim Dounin
http://mdounin.ru/