Fixing the well Documented Nginx Alias Traversal Vulnerability?

Thu Jul 13 15:35:44 UTC 2023

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/