You are viewing a plain text version of this content. The canonical link for it is here.
Posted to bugs@httpd.apache.org by bu...@apache.org on 2023/06/27 13:23:55 UTC

[Bug 66672] New: RewriteRule with QSA flag and substitution ending with ? broken

https://bz.apache.org/bugzilla/show_bug.cgi?id=66672

            Bug ID: 66672
           Summary: RewriteRule with QSA flag and substitution ending with
                    ? broken
           Product: Apache httpd-2
           Version: 2.4.57
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: mod_rewrite
          Assignee: bugs@httpd.apache.org
          Reporter: frank.meier@ergon.ch
  Target Milestone: ---

Created attachment 38592
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=38592&action=edit
fix RewriteRule with QSA flag and substitution ending with ?

after update from 2.4.55 to 2.4.57 we experience a change in behavior in our
regression tests: RewriteRules having the [QSA] flag set and the substitution
string ends with a question mark '?' (e.g 'RewriteRule "/do.php" "/it.php?"
[QSA]') do not append the query from the original URL anymore.

With 2.4.55 a query from the original request URL was appended to the resulting
URL. Now with 2.4.57 the query is always empty. 

If the substitution includes a parameter (e.g "/it.php?a=1") [QSA] flag works
as expected. 

I would argue, a substitution with a '?' at the end represents an 'empty'
query, where with the [QSA] flag set, the query from the original request must
be appended to.

The change was most probably introduced by this commit
https://svn.apache.org/viewvc?view=revision&revision=1908097.
I`ve written a small patch (attached) to revert the old behavior if the [QSA]
flag is set. This maybe to primitive but would fix our regression tests.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org

[Bug 66672] RewriteRule with QSA flag and substitution ending with ? broken

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=66672

Eric Covener <co...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |NEEDINFO

--- Comment #4 from Eric Covener <co...@gmail.com> ---
(In reply to Frank Meier from comment #3)
> Hi Eric,
> unfortunately the revised patch still leads to a regression in our test
> suite.
> 
> With RewriteRule: 
> `RewriteRule ^/mod_rewrite_before(.*)$ /mod_rewrite_after$1? [QSA,QSL]` 
> 
> and a request with encoded ? (%3f) in the path:
> `curl 'http://localhost/mod_rewrite_before/%3fxxx?param=123'`
> 
> leads now to splitting of the request path into the query. With 2.4.55: the
> the query was 'param=123' but with 2.4.57 (even with your patch) is:
> 'xxx&param=123'
> 
> In this case we use the '?' at the end of the substitution in combination
> with the [QSL] flag to secure the request from being split.
> 
> with mod_rewrite:trace8 i see the following difference in the logs:
> 2.4.55: `mod_rewrite.c(486): ... split uri=/mod_rewrite_after/?xxx? ->
> uri=/mod_rewrite_after/?xxx, args=param=123`
> 2.4.57: `mod_rewrite.c(493): ... split uri=/mod_rewrite_after/?xxx -> 
> uri=/mod_rewrite_after/, args=xxx&param=123`

Maybe we can tie this to QSL then, because that's what expects it to hang
around?

-        *(a2_end-1) = '\0'; /* trailing ? has done its job */
+        if (!(newrule->flags & RULEFLAG_QSLAST)) {
+            *(a2_end-1) = '\0'; /* trailing ? has done its job */
+        }

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org

[Bug 66672] RewriteRule with QSA flag and substitution ending with ? broken

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=66672

Eric Covener <co...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |FixedInTrunk

--- Comment #2 from Eric Covener <co...@gmail.com> ---
Committed with a minor tweak to remove the trailing ? in both cases. Will
propose to backport.

Added a testcase covering trailing ? and QSA together.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org

[Bug 66672] RewriteRule with QSA flag and substitution ending with ? broken

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=66672

--- Comment #3 from Frank Meier <fr...@ergon.ch> ---
Hi Eric,
unfortunately the revised patch still leads to a regression in our test suite.

With RewriteRule: 
`RewriteRule ^/mod_rewrite_before(.*)$ /mod_rewrite_after$1? [QSA,QSL]` 

and a request with encoded ? (%3f) in the path:
`curl 'http://localhost/mod_rewrite_before/%3fxxx?param=123'`

leads now to splitting of the request path into the query. With 2.4.55: the the
query was 'param=123' but with 2.4.57 (even with your patch) is:
'xxx&param=123'

In this case we use the '?' at the end of the substitution in combination with
the [QSL] flag to secure the request from being split.

with mod_rewrite:trace8 i see the following difference in the logs:
2.4.55: `mod_rewrite.c(486): ... split uri=/mod_rewrite_after/?xxx? ->
uri=/mod_rewrite_after/?xxx, args=param=123`
2.4.57: `mod_rewrite.c(493): ... split uri=/mod_rewrite_after/?xxx -> 
uri=/mod_rewrite_after/, args=xxx&param=123`

Disclaimer: We have written our own proxy module, so I am not entirely sure if
the issue does only exist for us. But I think it is a general problem.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org

[Bug 66672] RewriteRule with QSA flag and substitution ending with ? broken

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=66672

Christophe JAILLET <ch...@wanadoo.fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #10 from Christophe JAILLET <ch...@wanadoo.fr> ---
This was backported in 2.4.x branch in r1910999 and is part of version 2.4.58.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org

[Bug 66672] RewriteRule with QSA flag and substitution ending with ? broken

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=66672

Eric Covener <co...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEEDINFO                    |NEW

--- Comment #6 from Eric Covener <co...@gmail.com> ---
(In reply to Eric Covener from comment #5)
> I did an alternate addon fix in 1910650, making the flow look more like the
> the pre-CVE behavior.

back to your original patch now :)

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org

[Bug 66672] RewriteRule with QSA flag and substitution ending with ? broken

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=66672

--- Comment #8 from Frank Meier <fr...@ergon.ch> ---
Hi Eric,
I just wanted to let you know, that with your current patches from trunk
(1910662 + r1910666), we do not experience further failing regression tests.
Do you think this code is "final" and will be backported?

TL;DR
Maybe the code is not final, but there are some typos in the comments:
* `/* avoid getting a a query string via inadvertent capture */` -> `..getting
a query..`
* mentioned function `splitoutqueryargs` should be `splitout_queryargs` (two
occurrences)

Regards, Frank

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org

[Bug 66672] RewriteRule with QSA flag and substitution ending with ? broken

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=66672

--- Comment #5 from Eric Covener <co...@gmail.com> ---
I did an alternate addon fix in 1910650, making the flow look more like the the
pre-CVE behavior.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org

[Bug 66672] RewriteRule with QSA flag and substitution ending with ? broken

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=66672

--- Comment #7 from Eric Covener <co...@gmail.com> ---
Created attachment 38594
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=38594&action=edit
current diff

diff from last release

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org

[Bug 66672] RewriteRule with QSA flag and substitution ending with ? broken

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=66672

--- Comment #9 from Eric Covener <co...@gmail.com> ---
backported -- will be in 2.4.58

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org

[Bug 66672] RewriteRule with QSA flag and substitution ending with ? broken

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=66672

Eric Covener <co...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |PatchAvailable

--- Comment #1 from Eric Covener <co...@gmail.com> ---
Thanks Frank, checking it out

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org