Bug 62855 - Segfault in mod_include + printenv + ErrorDocument
Summary: Segfault in mod_include + printenv + ErrorDocument
Status: NEW
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: mod_include (show other bugs)
Version: 2.4.35
Hardware: PC Linux
: P2 normal (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
Keywords: PatchAvailable
Depends on:
Reported: 2018-10-25 12:11 UTC by Ewald Dieterich
Modified: 2018-11-21 15:06 UTC (History)
1 user (show)

Handle NULL environment values in mod_include.c, handle_printenv() (745 bytes, patch)
2018-10-25 12:11 UTC, Ewald Dieterich
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ewald Dieterich 2018-10-25 12:11:23 UTC
Created attachment 36214 [details]
Handle NULL environment values in mod_include.c, handle_printenv()

I configured mod_include for a location that serves local ErrorDocuments, like this:

ErrorDocument 400 /error/error.shtml
<Location "/error">
    AddType text/html .shtml
    AddOutputFilter INCLUDES .shtml
    Options +Includes

The error.shtml document uses the printenv directive and looks like this:

<!DOCTYPE html>
      <!--#printenv -->

Now I send an invalid request that leads to a "400 Bad Request" response:

echo "INVALID" | socket hostname 80

Apache segfaults in mod_include.c, handle_printenv() because the for loop in there assumes that every environment key also has a value. But in this scenario that's not the case for REDIRECT_REQUEST_METHOD, as there is no original REQUEST_METHOD. So the key REDIRECT_REQUEST_METHOD exists in r->subprocess_env, but its value is NULL.

I fixed this with the attached patch mod_include_printenv.patch by setting missing values to "ctx->intern->undefined_echo". This is what handle_echo() is doing, so I hope this makes sense. Or is simply skipping keys with missing values the better solution?
Comment 1 Micha Lenk 2018-10-25 13:01:36 UTC
The reason why the environment variable REDIRECT_REQUEST_METHOD exists but isn't set is because right before the internal redirect happens r->method is unconditionally backed up to the variable REQUEST_METHOD:

195              * This redirect needs to be a GET no matter what the original
196              * method was.
197              */
198             apr_table_setn(r->subprocess_env, "REQUEST_METHOD", r->method);

(File modules/http/http_request.c, SVN rev. 1844781)

Later in the function internal_internal_redirect this variable gets renamed to REDIRECT_REQUEST_METHOD without changing its value.