php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Bug #74670 Integer Underflow when unserializing GMP and possible other classes
Submitted: 2017-05-29 12:37 UTC Modified: 2018-07-02 21:42 UTC
From: emmanuel dot law at gmail dot com Assigned: nikic (profile)
Status: Closed Package: GNU MP related
PHP Version: 7.0.19 OS: *
Private report: No CVE-ID: None
View Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
If you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: emmanuel dot law at gmail dot com
New email:
PHP Version: OS:

 

 [2017-05-29 12:37 UTC] 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 ()

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2017-06-25 19:32 UTC] 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-06-25 22:03 UTC] 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-27 23:12 UTC] 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.
 [2018-07-02 15:33 UTC] nikic@php.net
-Status: Open +Status: Closed -Assigned To: +Assigned To: nikic
 [2018-07-02 15:33 UTC] nikic@php.net 
Fixed by https://github.com/php/php-src/commit/c793885b7624be4e2a95c69a2b8b3fee969b312f.
 [2018-07-02 21:42 UTC] stas@php.net
-Type: Security +Type: Bug
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Thu Nov 21 12:01:29 2024 UTC