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