Bug 48272 - mod_proxy_fcgi crashes Apache on invalid headers
Summary: mod_proxy_fcgi crashes Apache on invalid headers
Status: CLOSED FIXED
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: mod_proxy_fcgi (show other bugs)
Version: 2.5-HEAD
Hardware: PC Linux
: P2 critical with 3 votes (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-24 05:00 UTC by Edgar Frank
Modified: 2014-12-06 14:19 UTC (History)
1 user (show)



Attachments
Crash Backtrace (2.35 KB, text/plain)
2009-11-24 05:00 UTC, Edgar Frank
Details
GDB Backtrace (4.79 KB, text/plain)
2012-04-08 01:11 UTC, Blar
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Edgar Frank 2009-11-24 05:00:17 UTC
Created attachment 24604 [details]
Crash Backtrace

When mod_proxy_fcgi serves as a reverse proxy, it will crash the current
instance of apache with a segfault if it receives invalid headers
(invalid in terms of the ap_scan_script_header_err_core function in
server/util_script.c).

When mod_proxy_fcgi calls ap_scan_script_header_err_brigade with the
currently read data and ap_scan_script_header_err_core detects an
invalid header, the latter function will try to soak up all data in the
brigade.
As the given brigade does not contain an EOS bucket (this is added in
mod_proxy_fcgi only when the complete FCGI stream has been read from
the backend), getsfunc_BRIGADE will try to call the read function on a
brigade sentinel and crash.

Find attached a backtrace of the crash. The line-numbers aren't
unfortunately correct anymore, but finding the corresonding source
pieces should be straight forward.

Besides that mod_proxy_fcgi (IMHO) with its pipelining approach (as
 opposed to soaking the complete script output into memory before
forwarding it to the client) can't meet the preconditions to call
ap_scan_script_header_err_brigade, I'd suggest that getsfunc_BRIGADE
should be prepared in one way or another to see a brigade sentinel.

Futhermore ap_scan_script_header_err_core doesn't respect the possible
-1 return (TIMEOUT) from getsfunc_BRIGADE in the place where it soaks up
the invalid script output.

If the latter two suggestions will changed, I'd suggest this change also
for 2.2.x.

I filed this bug as PC/Linux as this is my current platform, but it
should be reproducable on any hardware/OS as far as I've seen it in the
source.

For your reference, this is the configure line I built Apache with:
./configure --with-mpm=worker --enable-so --enable-proxy
--enable-proxy-fcgi
with having APR and APU in srclib in version 1.3.9 and tested against
several trunk checkouts during the last month.

Platform is a Debian Lenny 64bit on an Intel QuadCore.
Comment 1 Blar 2012-04-08 01:11:17 UTC
Created attachment 28557 [details]
GDB Backtrace
Comment 2 Blar 2012-04-08 01:14:12 UTC
Same here. When i send a header('foo') from PHP-FPM (5.4) Apache 2.4 segfault with:

httpd[28201]: [core:notice] [pid 28201:tid 46912521880096] AH00052: child pid 28396 exit signal Segmentation fault (11), referer:
Comment 3 Stefan Fritsch 2012-04-09 09:46:14 UTC
Thanks for the detailed analysis, Edgar.

The crash because of the missing EOS bucket is fixed in trunk in r1311174. r1311172 fixes a related problem.

But I agree that mod_proxy_fcgi doesn't behave right and should read the complete headers before calling ap_scan_script_header_err_*
Comment 4 Jeff Trawick 2014-12-06 13:15:39 UTC
Note (to self?) this issue was also fixed in r1311174, and backported back to the 2.2.x branch:

"Futhermore ap_scan_script_header_err_core doesn't respect the possible
-1 return (TIMEOUT) from getsfunc_BRIGADE in the place where it soaks up
the invalid script output."
Comment 5 Jeff Trawick 2014-12-06 13:36:21 UTC
I don't see this issue in current or even fairly old levels of code:

"mod_proxy_fcgi doesn't behave right and should read the complete headers before calling ap_scan_script_header_err_*"

AFAICT mod_proxy_fcgi accumulates transient buckets of header until it detects the end, and only then does it call ap_scan_script_header_err_brigade_ex().

If anybody understands this differently, please open a separate bug; several issues have already been resolved under this one and are already released.
Comment 6 Yann Ylavic 2014-12-06 14:17:59 UTC
> But I agree that mod_proxy_fcgi doesn't behave right and should read the
> complete headers before calling ap_scan_script_header_err_*

Isn't that what is done with handle_headers()'s state machine?

This concern seems more appropriate for mod_cgi[d], but even there I don't see why ap_scan_script_header_err_brigade() could not read the headers by itself, it should stop on the first empty line anyway.

Btw, regarding proxy_fcgi, I don't see any more issue mentioned in this PR.
Comment 7 Yann Ylavic 2014-12-06 14:19:00 UTC
Oh, It seems you beat me on this :)