Bug 55323

Summary: Double unescaped uri in sub request handler
Product: Apache httpd-2 Reporter: Simon Klinkert <simon.klinkert>
Component: CoreAssignee: Apache HTTPD Bugs Mailing List <bugs>
Status: RESOLVED LATER    
Severity: normal Keywords: MassUpdate
Priority: P2    
Version: 2.2.20   
Target Milestone: ---   
Hardware: PC   
OS: All   
Attachments: re-escaping patch for ap_sub_req_method_uri

Description Simon Klinkert 2013-07-29 08:08:29 UTC
Created attachment 30643 [details]
re-escaping patch for ap_sub_req_method_uri

I'm observing a httpd/mod_webdav problem. When I try to upload or download a file and the file name contains the special character '%' (for example 'file%#12file'), then I get 400 and 403 errors every time.

After some debugging with dtrace and gdb, I assume the httpd unescapes the uri in ap_process_request_internal() more than once.

Take a look at this stack trace:

      httpd`ap_unescape_url
      httpd`ap_process_request_internal+0x107
      httpd`ap_sub_req_method_uri+0xdd
      httpd`ap_sub_req_lookup_uri+0x27
      mod_rewrite.so`lookup_variable+0x984
      mod_rewrite.so`do_expand+0xd2e
	splitout_queryargs
      mod_rewrite.so`apply_rewrite_list+0x354
      mod_rewrite.so`hook_uri2file+0x4f1
      httpd`ap_run_translate_name+0x2e
      httpd`ap_process_request_internal+0x2f5
      httpd`ap_process_request+0x165
      httpd`ap_process_http_connection+0x123
      httpd`ap_run_process_connection+0x2e
      httpd`child_main+0x493
      httpd`make_child+0x115
      httpd`startup_children+0x3d
      httpd`ap_mpm_run+0x90a
      httpd`main+0x9af
      httpd`_start+0x83

There are two calls of ap_process_request_internal() and thus two uri unescapes (ap_unescape_url()).

I'm not really familiar with the httpd code, but I guess a possible fix could re-escape the already unescaped uri in ap_sub_req_method_uri() before calling the internal request handler.

A little patch seems to be working for me. WebDav uploads and downloads for the file 'file%#12file' are now possible. I tested the version 2.2.20 but the 2.5 code looks broken as well.
Comment 1 Eric Covener 2013-07-29 11:17:41 UTC
That looks too general, your symptom is that you pass file%#12file into a rewriterule (-U check?) and it needs to be encoded at that point, but the patch would re-escape  anyone who sent in a properly escaped URL.
Comment 2 Simon Klinkert 2013-07-29 12:30:12 UTC
(In reply to Eric Covener from comment #1)
> That looks too general, your symptom is that you pass file%#12file into a
> rewriterule (-U check?) and it needs to be encoded at that point, but the
> patch would re-escape  anyone who sent in a properly escaped URL.

First of all, thanks for your feedback!

How do I encode the uri at "that point"?

There is no -U involved. My rewrite rule looks like this:

RewriteRule ^(/.*)? %{LA-U:ENV:storage_path}$1 [L]

In addition, I do not really see why this should be too general. The function ap_sub_req_method_uri does already a re-escaping if new_uri does not start with '/'. Why is that bad for the other case?
Comment 3 Eric Covener 2013-07-29 12:43:14 UTC
(In reply to Simon Klinkert from comment #2)
> (In reply to Eric Covener from comment #1)
> > That looks too general, your symptom is that you pass file%#12file into a
> > rewriterule (-U check?) and it needs to be encoded at that point, but the
> > patch would re-escape  anyone who sent in a properly escaped URL.
> 
> First of all, thanks for your feedback!
> 
> How do I encode the uri at "that point"?
> 
> There is no -U involved. My rewrite rule looks like this:
> 
> RewriteRule ^(/.*)? %{LA-U:ENV:storage_path}$1 [L]

$1 has captured a URL-escaped string. I think you can re-escabe it with the [B] flag, or pull the unescaped version out of %{THE_REQUEST} in a rewritecond.

> In addition, I do not really see why this should be too general. The
> function ap_sub_req_method_uri does already a re-escaping if new_uri does
> not start with '/'. Why is that bad for the other case?

Sorry, did not look to closely and not too familiar with it. Was just concerned generally about the scope of the change.
Comment 4 Eric Covener 2013-07-29 12:49:17 UTC
> $1 has captured a URL-escaped string. I think you can re-escabe it with the
> [B] flag, or pull the unescaped version out of %{THE_REQUEST} in a
> rewritecond.

Duh, I mean: $1 has captured an un-escaped
Comment 5 Simon Klinkert 2013-07-29 14:24:20 UTC
(In reply to Eric Covener from comment #4)
> > $1 has captured a URL-escaped string. I think you can re-escabe it with the
> > [B] flag, or pull the unescaped version out of %{THE_REQUEST} in a
> > rewritecond.
> 
> Duh, I mean: $1 has captured an un-escaped

The [B] flag didn't work for me (it has no effect), but I've no idea why. I will give it some further investigation later on.

%{THE_REQUEST} might work, but that's rather a workaround than a real fix.
Comment 6 Simon Klinkert 2013-08-05 13:17:16 UTC
Ok, I think the [B] flag doesn't work because there is no way to reach the backreference code path in function do_expand in my case. From the log files I can see that the double unescaping happens while we do a variable lookup for LA-U:ENV:storage_path. The variable lookup/subrequest is triggered by function do_expand as well. Maybe it's easier to understand if you take a look at my initially posted stack trace.

I think it's still ok to change the unescaping behavior in the sub request handler.
Comment 7 William A. Rowe Jr. 2018-11-07 21:09:38 UTC
Please help us to refine our list of open and current defects; this is a mass update of old and inactive Bugzilla reports which reflect user error, already resolved defects, and still-existing defects in httpd.

As repeatedly announced, the Apache HTTP Server Project has discontinued all development and patch review of the 2.2.x series of releases. The final release 2.2.34 was published in July 2017, and no further evaluation of bug reports or security risks will be considered or published for 2.2.x releases. All reports older than 2.4.x have been updated to status RESOLVED/LATER; no further action is expected unless the report still applies to a current version of httpd.

If your report represented a question or confusion about how to use an httpd feature, an unexpected server behavior, problems building or installing httpd, or working with an external component (a third party module, browser etc.) we ask you to start by bringing your question to the User Support and Discussion mailing list, see [https://httpd.apache.org/lists.html#http-users] for details. Include a link to this Bugzilla report for completeness with your question.

If your report was clearly a defect in httpd or a feature request, we ask that you retest using a modern httpd release (2.4.33 or later) released in the past year. If it can be reproduced, please reopen this bug and change the Version field above to the httpd version you have reconfirmed with.

Your help in identifying defects or enhancements still applicable to the current httpd server software release is greatly appreciated.