php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #72513 Stack-based buffer overflow vulnerability in virtual_file_ex
Submitted: 2016-06-29 06:09 UTC Modified: 2016-07-25 15:17 UTC
From: loianhtuan at gmail dot com Assigned: stas (profile)
Status: Closed Package: Filesystem function related
PHP Version: 7.1Git-2016-06-29 (Git) OS:
Private report: No CVE-ID: 2016-6289
 [2016-06-29 06:09 UTC] loianhtuan at gmail dot com
Description:
------------
CWD_API virtual_file_ex in php-src/Zend/zend_virtual_cwd.c has a bug that allow memcpy a large chunk of memory leads to buffer overflow.

The root cause of this vulnerability is integer overflow of path_length variable.

<snippet php-src/Zend/zend_virtual_cwd.c:1243>
```
CWD_API int virtual_file_ex(cwd_state *state, const char *path, verify_path_func verify_path, int use_realpath) /* {{{ */
{
	int path_length = (int)strlen(path);
	char resolved_path[MAXPATHLEN];
<...>
if (path_length == 0 || path_length >= MAXPATHLEN-1) { /*path_length can be overflow to negative value and passed this check*/
#ifdef ZEND_WIN32
		_set_errno(EINVAL);
#else
		errno = EINVAL;
#endif
		return 1;
	}
<...>
memcpy(resolved_path, path, path_length + 1);
```
</snippet>

PoC here is using ZipArchive to demonstrate the bug, but it may not be the only way.

I propose the following patch:

--- a/Zend/zend_virtual_cwd.c
+++ b/Zend/zend_virtual_cwd.c
@@ -1251,7 +1251,7 @@ CWD_API int virtual_file_ex(cwd_state *state, const char *path, verify_path_func
        int add_slash;
        void *tmp;

-       if (path_length == 0 || path_length >= MAXPATHLEN-1) {
+       if (path_length <= 0 || path_length >= MAXPATHLEN-1) {
 #ifdef ZEND_WIN32
                _set_errno(EINVAL);
 #else

Thanks!

Test script:
---------------
<?php
        ini_set('memory_limit',-1);
        $a = new ZipArchive();
        if ($a->open('a.zip', ZIPArchive::CREATE) !== TRUE){die("k9");};
        $a->extractTo(".", str_repeat("/",0xfffffff0));
        $a->close();
?>

Expected result:
----------------
No crash

Actual result:
--------------
Stopped reason: SIGSEGV
__memcpy_sse2_unaligned () at ../sysdeps/x86_64/multiarch/memcpy-sse2-unaligned.S:152
152     ../sysdeps/x86_64/multiarch/memcpy-sse2-unaligned.S: No such file or directory.
gdb-peda$ bt
#0  __memcpy_sse2_unaligned () at ../sysdeps/x86_64/multiarch/memcpy-sse2-unaligned.S:152
#1  0x0000000000885354 in virtual_file_ex (state=0x7fffffff7ee0, path=0x7ffef5a00018 '/' <repeats 200 times>...,
    verify_path=0x0, use_realpath=0x0) at /home/vps/git/php-src/Zend/zend_virtual_cwd.c:1326
#2  0x00007ffff5abc416 in php_zip_extract_file (za=0x11d5920, dest=0x7ffff6858e98 ".",
    file=0x7ffef5a00018 '/' <repeats 200 times>..., file_len=0xfffffff0) at /home/vps/git/php-src/ext/zip/php_zip.c:160
#3  0x00007ffff5ac28ae in c_ziparchive_extractTo (execute_data=0x7ffff6814120, return_value=0x7fffffffb110)
    at /home/vps/git/php-src/ext/zip/php_zip.c:2639
#4  0x00000000008a6a09 in ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER () at /home/vps/git/php-src/Zend/zend_vm_execute.h:973
#5  0x00000000008a53e9 in execute_ex (ex=0x7ffff6814030) at /home/vps/git/php-src/Zend/zend_vm_execute.h:428
#6  0x00000000008a54fb in zend_execute (op_array=0x7ffff687d000, return_value=0x0)
    at /home/vps/git/php-src/Zend/zend_vm_execute.h:473
#7  0x00000000008465ee in zend_execute_scripts (type=0x8, retval=0x0, file_count=0x3)
    at /home/vps/git/php-src/Zend/zend.c:1441
#8  0x00000000007b16a1 in php_execute_script (primary_file=0x7fffffffd6c0) at /home/vps/git/php-src/main/main.c:2515
#9  0x0000000000922222 in do_cli (argc=0x2, argv=0x10c3770) at /home/vps/git/php-src/sapi/cli/php_cli.c:993
#10 0x00000000009233e6 in main (argc=0x2, argv=0x10c3770) at /home/vps/git/php-src/sapi/cli/php_cli.c:1381
#11 0x00007ffff6faaf45 in __libc_start_main (main=0x922bde <main>, argc=0x2, argv=0x7fffffffea68, init=<optimized out>,
    fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffea58) at libc-start.c:287
#12 0x0000000000422ce9 in _start ()

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-06-29 07:21 UTC] pajoye@php.net
Thanks for the report :)

Fix is correct.

I wonder if we should not use size_t here as it clearly take random input from numerous places.
 [2016-06-29 07:49 UTC] loianhtuan at gmail dot com
Thank for you prompt feedback! :)
There is a code block later that use negative value so I'm not sure if we should change it to size_t.

<snippet php-src/Zend/zend_virtual_cwd.c:1380>
	path_length = tsrm_realpath_r(resolved_path, start, path_length, &ll, &t, use_realpath, 0, NULL);

	if (path_length < 0) {
		errno = ENOENT;
		return 1;
	}
</snippet>
 [2016-07-13 04:17 UTC] stas@php.net
Making it size_t will require also changing tsrm_realpath_r and struct _cwd_state (or insert checks) and maybe more things, so I don;t think the disruption is worth it. MAXPATHLEN is probably never longer than INT_MAX anyway and path this long can't be anything but wrong.
 [2016-07-13 04:18 UTC] stas@php.net
-Package: zip +Package: Filesystem function related
 [2016-07-13 04:53 UTC] stas@php.net
-Assigned To: +Assigned To: stas
 [2016-07-13 04:53 UTC] stas@php.net
fix in security repo as 56e260a2d0b6ecb5cd91357a579c04838be062a9
 [2016-07-14 02:32 UTC] loianhtuan at gmail dot com
Thanks for your time Stas. Btw, may I ask for a CVE number?
 [2016-07-19 09:01 UTC] stas@php.net
-Status: Assigned +Status: Closed
 [2016-07-19 09:01 UTC] stas@php.net
The fix for this bug has been committed.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.

 For Windows:

http://windows.php.net/snapshots/
 
Thank you for the report, and for helping us make PHP better.


 [2016-07-25 15:17 UTC] remi@php.net
-CVE-ID: +CVE-ID: 2016-6289
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Tue Mar 19 02:01:28 2024 UTC