Fixing the well Documented Nginx Alias Traversal Vulnerability?

Sat Jul 15 03:12:05 UTC 2023

Hello!

On Fri, Jul 14, 2023 at 06:50:54PM -0400, Jonathan Leitschuh wrote:

> Hi Maxim,
> 
> > The "i.." string is a valid path component and there are no reasons why
> it shouldn't be allowed - there might be such files and/or directories on
> the disk.
> 
> Do you have any metrics on how often this is truly used? Or how common this
> file path really is? Is the argument here that "this might be valid, so we
> should support it" or is there some wider use case of this standard that
> I'm unfamiliar with? It seems puzzling to me to support a potential file
> name like "i.." if it's frequency of use is incredibly low, and, more often
> than not, likely to result in a security vulnerability.

As a general-purpose web server, we've seen multiple cases of 
perfectly invalid URIs, yet used in real life, and users asking 
us to support them.  Examples include spaces in URIs 
(https://trac.nginx.org/nginx/ticket/196) and URIs with invalid 
%xx encoding.

As for the names like "i..", a trivial github search is as 
follows:

https://github.com/search?q=path%3A..%2F&type=code

As of now it returns about 136k results.

> > Similarly, location matching and alias as implemented in nginx (and
> documented) are expected to result in "../app/config.py" appended to
> "/data/w3/images/" for such a configuration.
> 
> Speaking as a security researcher, this seems blatantly like behaviour that
> most people would never want intentionally. The only valid use cases I
> could see would be as an intentional vulnerability introduced by someone
> building an intentionally vulnerable application for something like a
> security capture-the-flag. But in those cases, the vulnerability is
> intentionally left in place, instead of accidentally introduced. I don't,
> personally, have any concerns with breaking those cases. Are you aware of
> any valid uses cases of this behaviour in the real world, or is this
> behaviour left because of hypothetical configurations that this may break?

I don't think there are configurations which specifically designed 
to work with "../" to allow addressing a directory above the alias 
specified.  Though it is important to keep in mind that this is 
just an edge case of a generic prefix matching behaviour.  Prefix 
matching, which is often used to match specific directories, such 
as in

    location /images/ {
        ...
    }

can be also used to match arbitrary prefixes instead, and a 
configuration like (which is outlined to be a vulnerable 
configuration in the article you've linked):

    location /images {
        ...
    }

can be intentionally used to match not only "/images", "/images/", 
"/images/foo.jpg", and so on, but also "/images2", "/images2/", 
"/images2/bar.jpg", and so on.  And certainly there are multiple 
such configurations in place.

Further, most of the configurations using prefix matching without 
trailing slash aren't vulnerable, even if matching of additional 
URIs is not intentional, because access to relevant resources is 
anyway allowed.

> 
> Cheers,
> Jonathan Leitschuh
> 
> 
> On Thu, Jul 13, 2023 at 11:35 AM Maxim Dounin <mdounin at mdounin.ru> wrote:
> 
> > Hello!
> >
> > On Wed, Jul 12, 2023 at 05:45:34PM -0400, Jonathan Leitschuh wrote:
> >
> > > Hi Maxim,
> > >
> > > Pleasure to meet you!
> > >
> > > I've also included Munawar, the CEO of Open Refactory in this email
> > chain.
> > >
> > > > (It looks like you aren't subscribed to nginx-devel@, so the message
> > did
> > > not reach the mailing list: it only accepts messages from subscribers.
> > > I've preserved it in Cc: though, so we can continue discussion there,
> > with
> > > more developers involved.)
> > >
> > > Do I need to subscribe to continue to get this thread included in that
> > > email group? The website didn't explicitly say that I needed to subscribe
> > > to send emails there, that might be a good addition.
> >
> > The http://nginx.org/en/support.html page says:
> >
> > "To post to a mailing list, an e-mail address that will be used
> > for posting must first be subscribed."
> >
> > This applies to all mailing lists on the page, including
> > nginx-devel at .  Sorry if it wasn't clear.
> >
> > > > Indeed, the misconfiguration in question is a well-known one. Gixy, a
> > > tool to detect various nginx misconfigurations, tries to detect it at
> > least
> > > since 2017:
> > > >
> > > >
> > >
> > https://github.com/yandex/gixy/blob/master/docs/en/plugins/aliastraversal.md
> > >
> > > From the documentation, they provide the following example request:
> > > `/i../app/config.py`. I can't imagine a case where a request like
> > > `/i../app/config.py` could ever be made legitimately by a user. Why is
> > > `../app/config.py` appended to the alias `/data/w3/images/` in this case
> > > without nginx throwing an exception as this is a blatant path traversal
> > > attack?
> >
> > The "i.." string is a valid path component and there are no
> > reasons why it shouldn't be allowed - there might be such files
> > and/or directories on the disk.  Similarly, location matching
> > and alias as implemented in nginx (and documented) are expected to
> > result in "../app/config.py" appended to "/data/w3/images/" for
> > such a configuration.
> >
> > The question here is why the location prefix without trailing "/"
> > was used in the configuration, and if it was intentional.
> >
> > > > Unfortunately, I don't think there is an easy fix.  Changing the way
> > how
> > > nginx locations work would be a major change, and will break a lot of
> > valid
> > > configurations where prefix matching is intentionally used.
> > >
> > > This sounds like a fix that could this go through a depreciation cycle,
> > > where, nginx adds an intentional, opt-in, prefix-matching flag that, in
> > > it's early incarnations, offers warnings to users for
> > > deprecated configuration, and later, will fail-closed unless the flag is
> > > present?
> >
> > Sure, major changes can be implemented in a way which warns users
> > in advance.  This still be a major change though, and the
> > transition will require several years and will disturb a lot of
> > users, including ones who aren't affected by the configuration
> > issue in question.
> >
> > Further, it is not clear if it will help: nothing will stop users
> > from using the explicit flag for prefix matching in an unsafe way
> > without realizing it's unsafe.
> >
> > Given that, an explicit warning/note in the documentation looks
> > like the way to go, at least for now.
> >
> > >
> > > > If you think there is an easy fix, feel free to provide suggestions.
> > >
> > > On our side, between Munwar and myself, we have some significant
> > experience
> > > with bulk pull request generation to fix security vulnerabilities
> > at-scale
> > > across open source software. (See my Black Hat & DEF CON Talk on the
> > topic
> > > from last year
> > > <
> > https://github.com/jlleitschuh/#scaling-the-security-researcher-to-eliminate-oss-vulnerabilities-once-and-for-all
> > >
> > > )
> > >
> > > Munawar and his team at OpenRefactory did an analysis of the 2k+
> > > repositories that GitHub flags with the above linked search. From his
> > > team's analysis:
> > >
> > >  - Majority of the bugs are in documentation. These can be easily
> > filtered
> > > out with the file type. We can consider bulk pull request generation
> > adding
> > > a bit that we are fixing a documentation to make it correct so that
> > people
> > > are not misinformed and left with a vulnerability.
> > >  - There are a few bugs in the actual config files. In such cases, you
> > may
> > > split the projects based on the GH stars. The more popular ones should
> > get
> > > a stronger message communicating the risk to their end-users.
> > >
> > > We can likely massively clean-up this vulnerability across the OSS
> > > ecosystem, but the long-term solution to this vulnerability, I believe,
> > > will lie in the hands of the Nginx team.
> > >
> > > How can we work together here towards a world where this vulnerability is
> > > something end-users opt-in to, with intention (usually via some flag),
> > > rather than the world we are currently in, where this vulnerability
> > appears
> > > as a mistake?
> > >
> > > Cheers,
> > > Jonathan Leitschuh
> > >
> > >
> > > On Fri, Jul 7, 2023 at 7:21 PM Maxim Dounin <mdounin at mdounin.ru> wrote:
> > >
> > > > Hello!
> > > >
> > > > On Thu, Jul 06, 2023 at 12:05:25PM -0400, Jonathan Leitschuh wrote:
> > > >
> > > > > Hi Nginx Team,
> > > > >
> > > > > My name is Jonathan Leitschuh. I'm a Senior Software Security
> > Researcher
> > > > > for the Open Source Security Foundation: Project Alpha-Omega
> > > > > <https://openssf.org/community/alpha-omega/>. “Alpha” works with the
> > > > > maintainers of the most critical open source projects to help them
> > > > identify
> > > > > and fix security vulnerabilities, and improve their security posture.
> > > > > “Omega” identified at least 10,000 widely deployed OSS projects
> > where it
> > > > > can apply automated security analysis, scoring, and remediation
> > guidance
> > > > to
> > > > > their open source maintainer communities.
> > > > >
> > > > > Earlier today, on Twitter, I saw this discussion regarding
> > > > > the long-standing the NGinx Alias traversal vulnerability:
> > > > > https://twitter.com/infosec_au/status/1676072447156834304?s=20
> > > > >
> > > > > In particular, this discussion centered on this, this recent blog
> > post
> > > > > published on the 3rd of this month.
> > > > > https://labs.hakaioffsec.com/nginx-alias-traversal/
> > > > >
> > > > > In that blog post, the researchers detail how, due to
> > misconfiguration
> > > > and
> > > > > failing to specify the `/` character in the correct place can lead to
> > > > path
> > > > > traversal vulnerabilities.
> > > > >
> > > > >
> > > > >
> > > > > This vulnerability was previously discussed by Detectify in a blog
> > post
> > > > in
> > > > > 2020:
> > > > >
> > https://blog.detectify.com/2020/11/10/common-nginx-misconfigurations/
> > > > >
> > > > > This research originated with Orange Tsai back in 2018:
> > > > >
> > > >
> > https://i.blackhat.com/us-18/Wed-August-8/us-18-Orange-Tsai-Breaking-Parser-Logic-Take-Your-Path-Normalization-Off-And-Pop-0days-Out-2.pdf
> > > > >
> > > > > The recent hakaioffsec blog also identifies this case where the
> > > > > vulnerability also appears:
> > > > >
> > > > >
> > > > > My question to both the Nginx Security team, and the Nginx
> > Developers,
> > > > why
> > > > > not fix this vulnerability in Nginx itself such that it's safe by
> > > > default,
> > > > > but users can opt-in to the vulnerable behaviour if they so desire
> > > > through
> > > > > an explicit flag? Surely it would be fairly straightforward to
> > identify
> > > > > these vulnerable configurations and fix them for the user, or fail
> > closed
> > > > > (safe)?
> > > > >
> > > > > If you need evidence of the impact this vulnerability can have, the
> > > > > above hakaioffsec blog post details how this vulnerability resulted
> > in
> > > > > a US$6000 bounty from BitWarden.
> > > > >
> > > > > This GitHub search, proposed by the above blog post, returns 2.2k
> > results
> > > > > on GitHub:
> > > > >
> > > > >
> > > >
> > https://github.com/search?q=%2Flocation+%5C%2F%5B_.a-zA-Z0-9-%5C%2F%5D*%5B%5E%5C%2F%5D%5B%5Cs%5D%5C%7B%5B%5Cs%5Cn%5D*alias+%5C%2F%5B_.a-zA-Z0-9-%5C%2F%5D*%5C%2F%3B%2F+&type=code
> > > > >
> > > > > I'm curious what is currently preventing this from being fixed
> > upstream
> > > > to
> > > > > prevent end-users from accidentally implementing vulnerable
> > > > configurations
> > > > > like this.
> > > >
> > > > Thank you for your message.
> > > >
> > > > Indeed, the misconfiguration in question is a well-known one.
> > > > Gixy, a tool to detect various nginx misconfigurations, tries to
> > > > detect it at least since 2017:
> > > >
> > > >
> > > >
> > https://github.com/yandex/gixy/blob/master/docs/en/plugins/aliastraversal.md
> > > >
> > > > The root cause as I see it is that nginx locations (and alias)
> > > > work with prefix strings, while unaware people often expect them
> > > > to work with path components (directories) instead.  Note the
> > > > "Achieving Impact Without a Slash on Alias" section in the article
> > > > you've referenced: a configuration like
> > > >
> > > >     location /img {
> > > >         alias /var/images;
> > > >     }
> > > >
> > > > makes it possible to access "/var/images_confidential" via a
> > > > request to "/img_confidential".  This is because locations works
> > > > on prefix strings, and due to lack of trailing "/" in the location
> > > > prefix it does match "/img_confidential" request, and maps it to
> > > > the file system according to the configuration.
> > > >
> > > > Similarly, the same misconfiguration can happen even without the
> > > > "alias" directive at all. Consider the configuration:
> > > >
> > > >     location / { deny all; }
> > > >     location /images { allow all; }
> > > >
> > > > In contrast to some might (incorrectly) assume, a request to
> > > > "/images_confidential" will be allowed, and will return content as
> > > > per the configuration.
> > > >
> > > > Unfortunately, I don't think there is an easy fix.  Changing the
> > > > way how nginx locations work would be a major change, and will
> > > > break a lot of valid configurations where prefix matching is
> > > > intentionally used.
> > > >
> > > > Meanwhile, it surely deserves some additional warnings/notes in
> > > > the documentation, to emphasize the actual behaviour and potential
> > > > pitfalls.  Hopefully it would be enough to at least reduce the
> > > > amount of such misconfigurations.
> > > >
> > > > If you think there is an easy fix, feel free to provide
> > > > suggestions.
> > > >
> > > > (It looks like you aren't subscribed to nginx-devel@, so the
> > > > message did not reach the mailing list: it only accepts messages
> > > > from subscribers.  I've preserved it in Cc: though, so we can
> > > > continue discussion there, with more developers involved.)
> > > >
> > > > --
> > > > Maxim Dounin
> > > > http://mdounin.ru/
> > > >
> >
> > --
> > Maxim Dounin
> > http://mdounin.ru/
> >

-- 
Maxim Dounin
http://mdounin.ru/