httpd-bugs mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bugzi...@apache.org
Subject [Bug 53483] New: dangerous PCRE patterns in *Match directives
Date Fri, 29 Jun 2012 01:31:52 GMT
https://issues.apache.org/bugzilla/show_bug.cgi?id=53483

          Priority: P2
            Bug ID: 53483
          Assignee: bugs@httpd.apache.org
           Summary: dangerous PCRE patterns in *Match directives
          Severity: major
    Classification: Unclassified
                OS: All
          Reporter: calestyo@scientia.net
          Hardware: All
            Status: NEW
           Version: 2.5-HEAD
         Component: All
           Product: Apache httpd-2

Hi.

This is:

a) something that should definitely go to the release notes and announcement
mailing list


b) complicated to the complex handling and differences of whether request URIs
and especially directories have trailing / or not

c) I tried this so far only with Apache 2.2 and don't have an 2.4 right now
available.
I'm well aware of the avialability of $ in DirectoryMatch now, and the
documentation on it.

d) The documentation of the respective directives should be changed.

e) marked as major, as it easily grants access to things, that are not expected
to be accessible by the average users.




Now to the issue:


For all places where PCRE patterns are used, people will most usually want
them:

a) to start with "^"
=> otherwise the patterns match at any place

b) and end them with depending on the case (typically whether regular files or
directories should be matched) with "$" or "(?:$|/)"
=> other wise the pattern dont't end
=> my proposal, "(?:$|/)", works only (probably) well with Apache 2.4 ... more
on that later.


Before I go on all the directives:
The "?:" in "(?:$|/)" above means, that the subpattern (the "(...)" thingy) is
non capturing, i.e. you cannot use the match via e.g. \1 . This makes things
faster and one usually doesn't want to use $|/ later on.


The directives I cover below are likely not all affected; basically everything
which may have PCRE regexps as parameter might be.
Further I do not list the e.g. <Directory ~ "regexp"> forms, which are of
course affected, too.
I focus mainly on the directives which may have some file/dir access
controlling effect.


I) With respect to the "to start with '^'" thingy
AliasMatch:
Here you already clearly show the difference between patterns starting with ^
and those not.

<DirectoryMatch>:
The "^/www" example is safe, but please add another warning box, that notifies
about the differences between patterns starting with ^ and those not.

<FilesMatch>:
The "\.(gif|jpe?g|png)$" example is safe (or better said: what one expects),
though I'd suggest to change it to ".+\.(gif|jpe?g|png)$" as one usually don't
want to match the files ".gif", ".png", ".jpeg" or ".jpg" themselves.
But anyway please add another warning box, that notifies about the differences
between patterns starting with ^ and those not.

<LocationMatch>:
The example is "wrong" in a sense that it's not what people usually want
(matching the absolute server path). The text below says that already, but
perhaps mark the "contains" bold.
But anyway please add another warning box, that notifies about the differences
between patterns starting with ^ and those not.

RedirectMatch:
The "(.*)\.gif$" example is safe (or better said: what one expects), though I'd
suggest to change it to "/(.+)\.gif$" as one usually don't want to match the
file ".gif" themselves (the leading "/" is required, otherwise it would still
match ".gif" itself.
But anyway please add another warning box, that notifies about the differences
between patterns starting with ^ and those not.

ScriptAliasMatch:
The examples are safe (or better said: what one expects).
But anyway please add another warning box, that notifies about the differences
between patterns starting with ^ and those not.



Now so far things are not too bad, but now it gets problematic:
I) With respect to the "to end with '$' or '(?:$|/)'" thingy
AliasMatch:
The text says: "For example, to activate the /icons directory".
But actually the patterns also match any directories like /iconsFOOBAR.
So the first example is dangerous and tricky, it should be changed to:
"^/icons(/.*)(?:$|/)"
which matches:
/icons
/icons/
/icons/foobar
/icons/foobar/
but not:
/iconsFOOBAR
Analogously the case-insensitive example below.
The other examples are safe, as far as I can say:
But anyway please add another warning box, that notifies about the differences
between patterns ending with (?:$|/) and those not.

<DirectoryMatch>:
Here things are really dangerous:
The example "^/www/(.+/)?[0-9]{3}" is explained as "would match directories in
/www/ that consisted of three numbers".
Due to the (.+/)? this os wrong anyway as it must read "would match directories
in __OR SOMEWHERE BELOW__ /www/ that consisted of three numbers".
But it's even worse: as there is no ending of the pattern, it also matches
directories like:
"^/www/foobar/123BAZ"
Or try even a more simple example, e.g.
<DirectoryMatch "^/s">
Order Allow,Deny
Allow From All
</DirectoryMatch>
The directory "/secret" is matched too, and not just the directory "/s/.
The example must be changed to: "^/www/(.+/)?[0-9]{3}(?:$|/)"

Why is my "safe" end pattern (?:$|/) and not just /. Because we also want to
match directories that have no trailing / set.
As mentioned above, this should AFAIU work with apache 2.4, and needs the "$"
part.

Please add a red box, where you note people that this is different from how
<Directory> worked,... when the want to match full directory names (e.g. /123
and not also /123*) they need to close the pattern somehow, and take examples
like
/foo(?:$|/) means:
/foo
/foo/
/foo/bar
but not:
/fooBAZ
and
/foo(?:$|/$) or /foo/?$ means:
/foo
/foo/
but not:
/foo/bar
/fooBAZ

<FilesMatch>:
The example are safe, as far as I can say:
But anyway please add another warning box, that notifies about the differences
between patterns ending with $ and those not.

<LocationMatch>:
Analogous to <DirectoryMatch>...the example is dangerous, as not only
substrings /special/data but also /special/dataFOO would be matched,.. which is
correct from the wording, but people probably think that the later wouldn't be
matched.
So please add analogous info as above with <DirectoryMatch>.

RedirectMatch:
The exampl is safe, but anyway please add analogous info as above with
<DirectoryMatch>.

ScriptAliasMatch:
Analogous to <DirectoryMatch>...the example is dangerous, as not only
substrings /cgi-bin but also /cgi-binFOO would be matched,.. which is correct
from the wording, but people probably think that the later wouldn't be matched.
So please add analogous info as above with <DirectoryMatch>.



You see there is a lot of trickery in it. I don't doubt that you developers
know all these, but I guess for the average user, it's difficult to see from
the current documentation.


Actually there is even more trickery:
My "solutions" above, did not recongise that many slashes, are typically
collapsed to one, i.e. /foo////bar// is the same as /foo/bar/.
I'm not sure whether we should cover this or not, the above patterns would need
to be adapted with some quantifiers.


If you make changes, please post the commit IDs, so I can follow and verify :-)


Hope that helps and loads of thanks in advance,
Chris.

-- 
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


Mime
View raw message