php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #77367 Negative size parameter in mb_split
Submitted: 2018-12-29 02:43 UTC Modified: 2019-01-07 08:20 UTC
From: hugh at allthethings dot co dot nz Assigned: stas (profile)
Status: Closed Package: mbstring related
PHP Version: 7.3.0 OS: Linux
Private report: No CVE-ID: needed
 [2018-12-29 02:43 UTC] hugh at allthethings dot co dot nz
Description:
------------
mb_split doesn't correctly detect the length when the $string has an unfinished multibyte character at the end of the string. This causes a crash due to a negative parameter to add_next_index_stringl, which calls zend_string_init and memcpy.

This could be used to cause memory corruption/leakage.


configure options are:
./configure --enable-static --enable-cli --enable-mbstring --disable-shared --disable-all CFLAGS="-fPIC -Og -g3" LIBS=-ldl


Could reproduce on master. Couldn't reproduce on 5.6.39, 7.0.33, 7.1.25 and 7.2.13, though the same code exists (a different fuzz may produce the same).

Caused by comparison of unsigned number (size_t) to 0 to test whether it is positive. Fixed by altering the comparison to not test for negative, but instead check whether value to subtract is less than.

Patch at https://gist.github.com/hughdavenport/5128662f237d8b54d2e2b22dc7d18d5e

Interestingly, it doesn't crash with "[[:word:]]", but does with "\w", so the docs at https://secure.php.net/manual/en/regexp.reference.character-classes.php may be slightly incorrect.

Test script:
---------------
php -r 'var_dump(mb_split("\w","\xfc"));'

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


Actual result:
--------------
$ gdb --args ./php-7.3.0/sapi/cli/php-mbstring-afl-fast -r 'var_dump(mb_split("\w","\xfc"));'
GNU gdb (Ubuntu 8.1-0ubuntu3) 8.1.0.20180409-git
Copyright (C) 2018 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ./php-7.3.0/sapi/cli/php-mbstring-afl-fast...done.
(gdb) r
Starting program: /home/hugh/php-7.3.0/sapi/cli/php-mbstring-afl-fast -r var_dump\(mb_split\(\"\\w\",\"\\xfc\"\)\)\;
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".

Program received signal SIGSEGV, Segmentation fault.
__memmove_avx_unaligned_erms () at ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:526
526     ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S: No such file or directory.
(gdb) bt
#0  __memmove_avx_unaligned_erms () at ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:526
#1  0x0000000000a72a5a in zend_string_init (str=0x122821e "", len=18446744073709551611, persistent=0) at Zend/zend_string.h:157
#2  add_next_index_stringl (arg=0x7ffff661c080, str=0x122821e "", length=18446744073709551611) at Zend/zend_API.c:1595
#3  0x000000000072bf72 in zif_mb_split (execute_data=<optimized out>, return_value=<optimized out>) at ext/mbstring/php_mbregex.c:1303
#4  0x0000000000c4d418 in ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER (execute_data=0x7ffff661c030) at Zend/zend_vm_execute.h:690
#5  0x0000000000b3e39e in execute_ex (ex=0x7ffff661c030) at Zend/zend_vm_execute.h:55287
#6  0x0000000000b3e8db in zend_execute (op_array=0x7ffff667d2a0, return_value=<optimized out>) at Zend/zend_vm_execute.h:60834
#7  0x0000000000a36222 in zend_eval_stringl (str=<optimized out>, str_len=<optimized out>, retval_ptr=<optimized out>, string_name=<optimized out>) at Zend/zend_execute_API.c:1018
#8  0x0000000000a365fb in zend_eval_stringl_ex (str=0x121ae20 "var_dump(mb_split(\"\\w\",\"\\xfc\"));", str_len=140737327259816, retval_ptr=0x0, string_name=0xf0c436 "Command line code",
    handle_exceptions=1) at Zend/zend_execute_API.c:1059
#9  zend_eval_string_ex (str=0x121ae20 "var_dump(mb_split(\"\\w\",\"\\xfc\"));", retval_ptr=0xfffffffffffa1ef8, string_name=0x11ca0f6 <stage3_table_html5_1D500+774> "", handle_exceptions=1)
    at Zend/zend_execute_API.c:1070
#10 0x0000000000ce5c66 in do_cli (argc=<optimized out>, argv=<optimized out>) at sapi/cli/php_cli.c:1030
#11 0x0000000000ce407b in main (argc=3, argv=<optimized out>) at sapi/cli/php_cli.c:1392


$ ./php-7.3.0/sapi/cli/php-mbstring-asan-fast -r 'var_dump(mb_split("\w","\xfc"));'
=================================================================
==9942==ERROR: AddressSanitizer: negative-size-param: (size=-5)
    #0 0x4db2e5 in __asan_memcpy (/home/hugh/php-7.3.0/sapi/cli/php-mbstring-asan-fast+0x4db2e5)
    #1 0x1083844 in zend_string_init /home/hugh/php-7.3.0/Zend/zend_string.h:157:2
    #2 0x1083844 in add_next_index_stringl /home/hugh/php-7.3.0/Zend/zend_API.c:1595
    #3 0xa7648f in zif_mb_split /home/hugh/php-7.3.0/ext/mbstring/php_mbregex.c
    #4 0x1410c05 in ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER /home/hugh/php-7.3.0/Zend/zend_vm_execute.h:690:2
    #5 0x11fee0d in execute_ex /home/hugh/php-7.3.0/Zend/zend_vm_execute.h:55287:7
    #6 0x11ff661 in zend_execute /home/hugh/php-7.3.0/Zend/zend_vm_execute.h:60834:2
    #7 0x1012390 in zend_eval_stringl /home/hugh/php-7.3.0/Zend/zend_execute_API.c:1018:4
    #8 0x1012c1a in zend_eval_stringl_ex /home/hugh/php-7.3.0/Zend/zend_execute_API.c:1059:11
    #9 0x1012c1a in zend_eval_string_ex /home/hugh/php-7.3.0/Zend/zend_execute_API.c:1070
    #10 0x153d7c6 in do_cli /home/hugh/php-7.3.0/sapi/cli/php_cli.c:1030:5
    #11 0x153a6de in main /home/hugh/php-7.3.0/sapi/cli/php_cli.c:1392:18
    #12 0x7fb02af8cb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
    #13 0x43aa99 in _start (/home/hugh/php-7.3.0/sapi/cli/php-mbstring-asan-fast+0x43aa99)

0x6030000032fe is located 30 bytes inside of 32-byte region [0x6030000032e0,0x603000003300)
allocated by thread T0 here:
    #0 0x4f00f0 in malloc (/home/hugh/php-7.3.0/sapi/cli/php-mbstring-asan-fast+0x4f00f0)
    #1 0xf66f9c in __zend_malloc /home/hugh/php-7.3.0/Zend/zend_alloc.c:2904:14

SUMMARY: AddressSanitizer: negative-size-param (/home/hugh/php-7.3.0/sapi/cli/php-mbstring-asan-fast+0x4db2e5) in __asan_memcpy
==9942==ABORTING




Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2018-12-29 12:43 UTC] cmb@php.net
-Status: Open +Status: Verified -Assigned To: +Assigned To: cmb
 [2018-12-29 12:43 UTC] cmb@php.net
Thanks for reporting this bug.  I can confirm the described issue,
and also that your patch fixes it.  PHP-7.2 and older are not affected,
since `n` had been declared as int there.

> Interestingly, it doesn't crash with "[[:word:]]", but does with
> "\w", so the docs at
> https://secure.php.net/manual/en/regexp.reference.character-classes.php
> may be slightly incorrect.

These docs are about the PCRE extension; mb_split() and the other
mbregex functions use the Oniguruma engine which uses a somewhat
different pattern syntax.
 [2018-12-29 13:22 UTC] cmb@php.net
-Status: Verified +Status: Analyzed -Assigned To: cmb +Assigned To: stas
 [2018-12-29 13:22 UTC] cmb@php.net
Stas, can you please apply the formatted patch[1] to the security
repo (PHP-7.3 and up)?

[1] <https://gist.github.com/cmb69/a2be4dd4240bbee11ce4cb801c60d6ab>
 [2018-12-29 19:42 UTC] hugh at allthethings dot co dot nz
Ah yeh, being an int in previous versions makes sense. Thought I was going mad!

That makes sense with the docs. I'm not sure any of the function pages describe what syntax Oniguruma uses. Would it be worth me creating another bug for that to get written at some point?

Cheers,

Hugh
 [2018-12-30 03:12 UTC] stas@php.net
-CVE-ID: +CVE-ID: needed
 [2018-12-30 03:12 UTC] stas@php.net
In security repo as ee30a1fa4c05a5e3d53898296ed9bdb6a1d63239
 [2018-12-30 22:36 UTC] cmb@php.net
> That makes sense with the docs. I'm not sure any of the function
> pages describe what syntax Oniguruma uses. Would it be worth me
> creating another bug for that to get written at some point?

A quick search for existing Oniguruma pattern syntax in the manual
brought up nothing, so indeed this should be filed as
documentation problem.  The syntax is documented in
<https://github.com/kkos/oniguruma/blob/master/doc/RE>.
 [2018-12-30 22:42 UTC] hugh at allthethings dot co dot nz
File bug #77383 for the doc issue
 [2019-01-07 08:20 UTC] stas@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=e617f03066ce81d26f56c06d6bd7787c7de08703
Log: Fix #77367: Negative size parameter in mb_split
 [2019-01-07 08:20 UTC] stas@php.net
-Status: Analyzed +Status: Closed
 [2019-01-07 08:21 UTC] stas@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=e617f03066ce81d26f56c06d6bd7787c7de08703
Log: Fix #77367: Negative size parameter in mb_split
 [2019-01-07 13:17 UTC] cmb@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=11ce508ee3390d4e68542c9fdae1277e3e75a573
Log: Fix #77367: Negative size parameter in mb_split
 
PHP Copyright © 2001-2019 The PHP Group
All rights reserved.
Last updated: Thu Jan 17 19:01:26 2019 UTC