Sec Bug->Bug #74670 [Csd]: Integer Underflow when unserializing GMP and possible other classes

Edit report at https://bugs.php.net/bug.php?id=74670&edit=1 ID: 74670 Updated by: stas@php.net Reported by: emmanuel dot law at gmail dot com Summary: Integer Underflow when unserializing GMP and possible other classes Status: Closed -Type: Security +Type: Bug Package: GNU MP related Operating System: * PHP Version: 7.0.19 Assigned To: nikic Block user comment: N Private report: N Previous Comments: ------------------------------------------------------------------------ [2018-07-02 15:33:34] nikic@php.net Fixed by https://github.com/php/php-src/commit/c793885b7624be4e2a95c69a2b8b3fee969b312f. ------------------------------------------------------------------------ [2017-06-27 23:12:31] nikic@php.net The missing null termination allows the YYCURSOR to advance past max, which causes the incorrect computation of maxlen in the code you quoted in the initial description. As the null termination issue is unlikely to get fixed (it's an re2c limitation without a clear workaround), what we could do here is move the check from finish_nested_data to before the ->unserialize() call. This would ensure that the string that is passed to ->unserialize() it "terminated" by a "}", which will probably have a similar practical effect as null termination. Another possibility is to copy and terminate the string before passing it to ->unserialize(). This might be a good option for PHP 7.2, in combination with a change of the API to use zend_string. ------------------------------------------------------------------------ [2017-06-25 22:03:49] emmanuel dot law at gmail dot com I can't see #69429 since it's locked. But I'm not sure if lack of NULL byte is the problem in this case. Rather I think the problem is that datalen is user controllable which gets fed into ce->unserialize. Maybe we are talking about the same thing, but I cant be sure without looking at #69429. However I think you are right in that switching GMP over to use Serializable methods would solve this particular instance of vulnerability. However any future (or even existing custom ) classes that uses class handlers directly would be be susceptible. How/Do we need to prevent future classess from directly implements the unserialize() handler? ------------------------------------------------------------------------ [2017-06-25 19:32:13] nikic@php.net I believe the root cause of this issue is bug #69429: The unserializer currently assumes that the string being unserialized is NUL terminated. The GMP unserializer is one of the few (the only?) cases where this is not a given, because it directly implements the unserialize() handler. Most other classes instead go through the Serializable interface, in which case they will receive a NUL-terminated copy of the string. The short term fix for this would be to switch GMP over to use Serializable methods, rather than directly using the class handlers. ------------------------------------------------------------------------ [2017-05-29 12:37:50] emmanuel dot law at gmail dot com Description: ------------ There is an integer underflow when unserializing a specially crafted malform GMP: C:3:"GMP":4:{s:6666666666:""} This results in a Segmentation fault When unserializing GMP, it's expecting to process 4 characters, however when parsing "s:6666666666", this results in an integer underflow at line 1354 below, where YYCURSOR > max. This results in maxlen being a large unsigned integer > len. php-7.0.19/ext/standard/var_unserializer.c:1354 1349 { 1350 size_t len, maxlen; 1351 char *str; 1352 1353 len = parse_uiv(start + 2); 1354 maxlen = max - YYCURSOR; 1355 if (maxlen < len) { 1356 *p = start + 2; 1357 return 0; I think the root cause of the issue is in object_custom() at php-7.0.19/ext/standard/var_unserializer.c:438. This is because datalen is obtained from a user-controlled value(line 442) which is then fed into ce->unserialize (line 454) 438 static inline int object_custom(UNSERIALIZE_PARAMETER, zend_class_entry *ce) 439 { ..... 442 datalen = parse_iv2((*p) + 2, p); ...... 454 } else if (ce->unserialize(rval, ce, (const unsigned char*)*p, datalen, (zend_unserialize_data *)var_hash) != SUCCESS) { I suspect any other class that uses custom object unserializsation could possibly be affected. One of the impact of this vulnerability would be similar to https://bugs.php.net/bug.php?id=69324 where one can leak/dump memory data, a la heartbleed style. Test script: --------------- <?php unserialize('C:3:"GMP":4:{s:6666666666:""}'); Actual result: -------------- ,/php-7.0.19 unserialized_GMP.php Segmentation fault ====backtrace==== gdb-peda$ bt #0 0x00000000009dde84 in php_var_unserialize_internal (rval=0x7fffec476010, p=0x7fffffffa118, max=0x7fffec456291 "6666666:\"\"}\n", var_hash=0x7fffffffa120, classes=0x0) at /root/php-7.0.19/ext/standard/var_unserializer.c:1364 #1 0x00000000009dad41 in php_var_unserialize_ex (rval=0x7fffec476010, p=0x7fffffffa118, max=0x7fffec456291 "6666666:\"\"}\n", var_hash=0x7fffffffa120, classes=0x0) at /root/php-7.0.19/ext/standard/var_unserializer.c:539 #2 0x00000000009dacd8 in php_var_unserialize (rval=0x7fffec476010, p=0x7fffffffa118, max=0x7fffec456291 "6666666:\"\"}\n", var_hash=0x7fffffffa120) at /root/php-7.0.19/ext/standard/var_unserializer.c:530 #3 0x00000000006c0441 in gmp_unserialize (object=0x7fffec476000, ce=0x1685830, buf=0x7fffec45628d "s:666666666:\"\"}\n", buf_len=0x4, data=0x7fffffffa6d8) at /root/php-7.0.19/ext/gmp/gmp.c:604 #4 0x00000000009da291 in object_custom (rval=0x7fffec476000, p=0x7fffffffa6d0, max=0x7fffec45629d "", var_hash=0x7fffffffa6d8, classes=0x0, ce=0x1685830) at /root/php-7.0.19/ext/standard/var_unserializer.c:454 #5 0x00000000009dd08d in php_var_unserialize_internal (rval=0x7fffec476000, p=0x7fffffffa6d0, max=0x7fffec45629d "", var_hash=0x7fffffffa6d8, classes=0x0) at /root/php-7.0.19/ext/standard/var_unserializer.c:1230 #6 0x00000000009dad41 in php_var_unserialize_ex (rval=0x7fffec476000, p=0x7fffffffa6d0, max=0x7fffec45629d "", var_hash=0x7fffffffa6d8, classes=0x0) at /root/php-7.0.19/ext/standard/var_unserializer.c:539 #7 0x00000000009c43da in zif_unserialize (execute_data=0x7fffec4130d0, return_value=0x7fffec4130c0) at /root/php-7.0.19/ext/standard/var.c:1076 #8 0x0000000000b7fd37 in ZEND_DO_ICALL_SPEC_HANDLER () at /root/php-7.0.19/Zend/zend_vm_execute.h:586 #9 0x0000000000b7e4a6 in execute_ex (ex=0x7fffec413030) at /root/php-7.0.19/Zend/zend_vm_execute.h:414 #10 0x0000000000b7edf1 in zend_execute (op_array=0x7fffec47e000, return_value=0x0) at /root/php-7.0.19/Zend/zend_vm_execute.h:458 #11 0x0000000000aeb2d1 in zend_execute_scripts (type=0x8, retval=0x0, file_count=0x3) at /root/php-7.0.19/Zend/zend.c:1443 #12 0x0000000000a38fca in php_execute_script (primary_file=0x7fffffffd0e0) at /root/php-7.0.19/main/main.c:2492 #13 0x0000000000c5ef01 in do_cli (argc=0x3, argv=0x15b4540) at /root/php-7.0.19/sapi/cli/php_cli.c:977 #14 0x0000000000c602d7 in main (argc=0x3, argv=0x15b4540) at /root/php-7.0.19/sapi/cli/php_cli.c:1347 #15 0x00007ffff33ad830 in __libc_start_main (main=0xc5facc <main>, argc=0x3, argv=0x7fffffffe468, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffe458) at ../csu/libc-start.c:291 #16 0x0000000000445af9 in _start () ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=74670&edit=1