merge_slashes and decoding of the path

[Maxim Dounin]

Hello!

On Mon, Nov 25, 2013 at 11:30:46AM -0600, Jason Barnabe wrote:

> On Mon, Nov 25, 2013 at 11:08 AM, Maxim Dounin <mdounin at mdounin.ru> wrote:
> 
> > Example of a vulnerable configuration is given in the directive
> > description you've linked (http://nginx.org/r/merge_slashes):
> >
> > : Note that compression is essential for the correct matching of
> > : prefix string and regular expression locations. Without it, the
> > : “//scripts/one.php” request would not match
> > :
> > : location /scripts/ {
> > :     ...
> > : }
> > : and might be processed as a static file. So it gets converted to
> > : “/scripts/one.php”.
> >
> > That is, with merge_slashes switched off, restrictions like
> >
> >     location /protected/ {
> >         deny all;
> >     }
> >
> > can be easily bypassed by using a request to "//protected/file".
> > It should be taken into account while writing a configuration for
> > a server with merge_slashes switched off.
> >
> 
> I'm not sure that applies in my configuration, where I'm using Passenger
> and have no "protected" locations, but I can see how this could lead to
> problems.
> 
> 
> > > Why does nginx not allow the
> > > encoded slashes to be passed through (like Apache does[2]), and if it did
> > > so, would that negate the security concerns?
> >
> > While not decoding slashes is probably a better than not merging
> > them, it's not really a good aproach either.  This way, the
> >
> >     http://www.mysite.com/search/http%3A%2F%2Fexample.com%2F
> >
> > URL becomes equivalent to
> >
> >     http://www.mysite.com/search/http%3A%252F%252Fexample.com%252F
> >
> > which isn't really consistent and may produce unexpected results.
> >
> 
> I don't see how this would necessarily be the case, but I'll defer to your
> knowledge. However, I don't think we should let perfect be the enemy of
> "better", especially if the thing "better" would replace is a potential
> security concern.

E.g., consider a configuration using an imaginary "decode_slashes" 
directive:

    decode_slashes off;
    
    location / {
        proxy_pass http://backend;
    }

    location /protected/ {
        auth_basic ...
        proxy_pass http://backend;
    }

A request to "/protected%2Ffile" would be passed to a backend with 
non-decoded slash, and if there is an nginx in default 
configuration, %2F will be decoded there.  That is, a 
"/protected/file" will be returned, bypassing security 
restrictions configured.

Given the above, I think that not decoding slashes isn't a 
solution.  It may be useful in some configurations, but it isn't 
safe from security point of view either.

> I do have this worked around in my app code (look for http:/, replace with
> http://) so it's not a functional issue for me at the moment. It would be a
> nice feature if it could be done sensibly - should I file an issue in trac?

I don't think that we need this feature due to inconsistencies it 
introduce, see above.

-- 
Maxim Dounin
http://nginx.org/en/donation.html