Curl does not limit output data size

[ElliotKillick]

I did this

Consider the following: test="$(curl https://example.com)"

example.com could cause curl to download a very large amount of data which could lead to an out of memory scenario when put into the shell variable test.

A similar DoS could happen if writing to the disk by filling up disk space to its entirety.

Both of these could completely lock up a system and cause other applications to abort (or crash if they're written poorly). For memory exhaustion, a system with OOM reaper may also eventually take action.

I expected the following

Considering this is such a basic security issue, curl should have a built-in option to limit its output size from stdout/-o/--output. Once the hard limit for output size has been reached, curl should stop reading from the network and terminate with curl error 23 or probably better would be a new error code to indicate this exact error.

I propose one of --max-size (similar to --max-time), --max-output, or --max-out-size (similar to --max-filesize which doesn't serve as a hard limit) to stay aligned with option names already existing in curl.

curl/libcurl version

curl 8.0.1

operating system

Fedora Linux 38

[dfandrich]

The Unix way is to pipe the output of curl into head or dd or some other program to limit the maximum amount of data if that is an issue in your application. --max-filesize will prevent a too-long download from even starting, which is an optimization that a pipe can't (easily) do, but it's easy to limit the data transferred without adding the 258th command-line option to curl.

[ElliotKillick]

--max-filesize won't work as a hard limit due to the reason given in the curl manual NOTE for this option. Changing this option to also enforce a hard limit would be an acceptable solution.

For Unix systems, head -c mostly works. Although because it was only recently added to POSIX, not all Unix systems have this yet, for example, OpenBSD. dd would definitely work on all Unix systems in practice, however, due to how dd works it may come with a performance hit (not to mention it's not pretty).

I've tried piping into head -c, however, in POSIX sh it can't easily be done when you also want to check curl's error code. This is because head (the last command run in the pipeline) will conceal curl's error code. set -o pipefail can be used to propagate curl's error code but again, only recently added to POSIX and popular shells like Dash have yet to support it.

Moreover, I think this security feature should be doable in curl without requiring a shell. The above doesn't cover Windows of course.

[bagder]

Maybe we should make --max-filesize also work as a secondary check to stop "too much" data from being saved.

[jay]

diff --git a/src/tool_cb_wrt.c b/src/tool_cb_wrt.c
index 2f8c6ac..e662a35 100644
--- a/src/tool_cb_wrt.c
+++ b/src/tool_cb_wrt.c
@@ -231,6 +231,10 @@ size_t tool_write_cb(char *buffer, size_t sz, size_t nmemb, void *userdata)
     }
   }
 
+  if(config->max_filesize && (outs->bytes + bytes > config->max_filesize)) {
+    return CURLE_FILESIZE_EXCEEDED;
+  }
+
 #ifdef WIN32
   fhnd = _get_osfhandle(fileno(outs->stream));
   /* if windows console then UTF-8 must be converted to UTF-16 */

It's documented to do nothing if the file size is not known but I think there's flexibility to remove that because of the way that option is used.

[bagder]

I think there's flexibility to remove that because of the way that option is used.

I think so too. Will you make a PR out this and we can give it a go?

[ElliotKillick]

Thank you curl team for the fast response and turnaround on this issue!

I find the description of this issue coincides with CWE-400. Do you think it's worth advising users with a minor CVE? Due to curl's popularity, there are probably cases (e.g. on an embedded device with few resources and no GNU coreutils/Bash) where an automated script could run into this weakness with no easily portable way to fix it on their side until now. Thanks so much again.

[dfandrich]

It's working as documented, so I wouldn't consider this a security problem. There are ways to avoid filling up a storage partition without this feature, so if this could have been an issue people could have used other means.

[ElliotKillick]

Would you consider it a security problem in the application then due to it using curl incorrectly?

Sure, there's always going to be other means in the form of some hack. However, the fact is that most people aren't going to do that and as a result write insecure applications.

[dfandrich]

There may be security problems in applications using curl, no question. curl gives people 1001 ways to shoot themselves in the foot if they so desire. That's why we have a libcurl-security document to try to help application authors to avoid doing so. This exact issue is mentioned there, in fact (see the Denial of Service section). Given that curl is working exactly as advertised in this case, and it's advertising just what we expect it to, I don't see what we'd be raising a security vulnerability against.

[ElliotKillick]

From the libcurl-security document:

Instead, applications should monitor the amount of data received within the write or progress callback and abort once the limit is reached.

This document only provides a solution for libcurl not the curl CLI tool.

My point is that there was an unreasonably large expectation on developers using the curl command to come up with some hack for solving this issue due to none being documented and curl's popularity on many platforms. This is why I think a CVE makes sense. It would be beneficial to organizations using curl because I'm willing to bet that most scripts aren't doing this possibly at little to no fault of their own.

While curl may have been working as advertised, it didn't give developers the functionality needed to defend against the documented DoS threat, nor was there any robust way to get that functionality by piping into external programs in some cases.

[bagder]

Several curl team members have repeated the same message many times:

this is curl working as advertised

If this causes a security problem, then that problem is in the user's backyard and you should file a CVE for that product/service. It is not a security problem for curl itself.

Filing a CVE for curl working as intended and documented is not helpful to anyone.

[ElliotKillick]

I believe there's been a misunderstanding. The fact that curl's default behavior is working as advertised was never the issue. The issue is that curl had no option to set a maximum output size if the developer desired it.

Furthermore, there is no robust (implemented POSIX) solution to the problem should a developer want to include this security control it in their script. It's like a known vulnerability with no patch or mitigation available for all platforms. Prior to this issue, if I were to raise this problem for another product/service as you recommended then I may not have had any patch that I could give the dev team to fix this issue.

If the curl tool is handling an untrusted URL, then this is a security problem because it affects availability, which is part of the CVSS framework.

Not having this security control always be possible is unacceptable because curl is often responsible for interacting with data from the most untrusted source there is - the Internet. Maybe that was acceptable at one point, but in 2023, it's not.

[bagder]

You can continue to repeat your stance, and we can continue to repeat ours. It's not very productive though.