Skip to content

Commit c793885

Browse files
nikicnikic
committed
Fixed bug #74670
Validate that "C" serialization payload is followed by "}" prior to calling the unserialize() handler. This mitigates issues caused by unserialize() not correctly handling strings that are not NUL terminated. Making sure that there is a "}" at the end avoids the problem.
1 parent f825832 commit c793885

File tree

7 files changed

+67
-41
lines changed

7 files changed

+67
-41
lines changed

‎NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ PHP NEWS
1515
. Fixed bug #73342 (Vulnerability in php-fpm by changing stdin to
1616
non-blocking). (Nikita)
1717

18+
- GMP:
19+
. Fixed bug #76470 (Integer Underflow when unserializing GMP and possible
20+
other classes). (Nikita)
21+
1822
- intl:
1923
. Fixed bug #76556 (get_debug_info handler for BreakIterator shows wrong
2024
type). (cmb)

‎ext/gmp/bug74670.phpt

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
--TEST--
2+
Bug #74670: Integer Underflow when unserializing GMP and possible other classes
3+
--FILE--
4+
<?php
5+
$str = 'C:3:"GMP":4:{s:6666666666:""}';
6+
var_dump(unserialize($str));
7+
?>
8+
--EXPECTF--
9+
Notice: unserialize(): Error at offset 13 of 29 bytes in %s on line %d
10+
bool(false)

‎ext/gmp/tests/serialize.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ try {
1818
} catch (Exception $e) { var_dump($e->getMessage()); }
1919

2020
try {
21-
unserialize('C:3:"GMP":8:{s:2:"42";}');
21+
unserialize('C:3:"GMP":9:{s:2:"42";}');
2222
} catch (Exception $e) { var_dump($e->getMessage()); }
2323

2424
?>

‎ext/standard/tests/serialize/bug71311.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
Bug #71311 Use-after-free vulnerability in SPL(ArrayObject, unserialize)
33
--FILE--
44
<?php
5-
$data = unserialize("C:11:\"ArrayObject\":11:{x:i:0;r:3;XX");
5+
$data = unserialize("C:11:\"ArrayObject\":11:{x:i:0;r:3;X}");
66
var_dump($data);
77
?>
88
--EXPECTF--

‎ext/standard/tests/serialize/bug73341.phpt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ Bug #73144 (Use-afte-free in ArrayObject Deserialization)
33
--FILE--
44
<?php
55
try {
6-
$token = 'a:2:{i:0;O:1:"0":2:0s:1:"0";i:0;s:1:"0";a:1:{i:0;C:11:"ArrayObject":7:0x:i:0;r0';
6+
$token = 'a:2:{i:0;O:1:"0":2:0s:1:"0";i:0;s:1:"0";a:1:{i:0;C:11:"ArrayObject":7:{x:i:0;r}';
77
$obj = unserialize($token);
88
} catch(Exception $e) {
99
echo $e->getMessage()."\n";
@@ -21,4 +21,4 @@ unserialize($exploit);
2121
Error at offset 6 of 7 bytes
2222

2323
Notice: ArrayObject::unserialize(): Unexpected end of serialized data in %sbug73341.php on line %d
24-
Error at offset 24 of 34 bytes
24+
Error at offset 24 of 34 bytes

‎ext/standard/var_unserializer.c

Lines changed: 40 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* Generated by re2c 0.16 */
1+
/* Generated by re2c 1.0.1 */
22
#line 1 "ext/standard/var_unserializer.re"
33
/*
44
+----------------------------------------------------------------------+
@@ -477,16 +477,22 @@ static inline int object_custom(UNSERIALIZE_PARAMETER, zend_class_entry *ce)
477477
return 0;
478478
}
479479

480+
/* Check that '}' is present before calling ce->unserialize() to mitigate issues
481+
* with unserialize reading past the end of the passed buffer if the string is not
482+
* appropriately terminated (usually NUL terminated, but '}' is also sufficient.) */
483+
if ((*p)[datalen] != '}') {
484+
return 0;
485+
}
486+
480487
if (ce->unserialize == NULL) {
481488
zend_error(E_WARNING, "Class %s has no unserializer", ZSTR_VAL(ce->name));
482489
object_init_ex(rval, ce);
483490
} else if (ce->unserialize(rval, ce, (const unsigned char*)*p, datalen, (zend_unserialize_data *)var_hash) != SUCCESS) {
484491
return 0;
485492
}
486493

487-
(*p) += datalen;
488-
489-
return finish_nested_data(UNSERIALIZE_PASSTHRU);
494+
(*p) += datalen + 1; /* +1 for '}' */
495+
return 1;
490496
}
491497

492498
static inline zend_long object_common1(UNSERIALIZE_PARAMETER, zend_class_entry *ce)
@@ -603,7 +609,7 @@ static int php_var_unserialize_internal(UNSERIALIZE_PARAMETER)
603609
start = cursor;
604610

605611

606-
#line 607 "ext/standard/var_unserializer.c"
612+
#line 613 "ext/standard/var_unserializer.c"
607613
{
608614
YYCTYPE yych;
609615
static const unsigned char yybm[] = {
@@ -661,9 +667,9 @@ static int php_var_unserialize_internal(UNSERIALIZE_PARAMETER)
661667
yy2:
662668
++YYCURSOR;
663669
yy3:
664-
#line 982 "ext/standard/var_unserializer.re"
670+
#line 988 "ext/standard/var_unserializer.re"
665671
{ return 0; }
666-
#line 667 "ext/standard/var_unserializer.c"
672+
#line 673 "ext/standard/var_unserializer.c"
667673
yy4:
668674
yych = *(YYMARKER = ++YYCURSOR);
669675
if (yych == ':') goto yy17;
@@ -710,13 +716,13 @@ static int php_var_unserialize_internal(UNSERIALIZE_PARAMETER)
710716
goto yy3;
711717
yy15:
712718
++YYCURSOR;
713-
#line 976 "ext/standard/var_unserializer.re"
719+
#line 982 "ext/standard/var_unserializer.re"
714720
{
715721
/* this is the case where we have less data than planned */
716722
php_error_docref(NULL, E_NOTICE, "Unexpected end of serialized data");
717723
return 0; /* not sure if it should be 0 or 1 here? */
718724
}
719-
#line 720 "ext/standard/var_unserializer.c"
725+
#line 726 "ext/standard/var_unserializer.c"
720726
yy17:
721727
yych = *++YYCURSOR;
722728
if (yybm[0+yych] & 128) {
@@ -728,13 +734,13 @@ static int php_var_unserialize_internal(UNSERIALIZE_PARAMETER)
728734
goto yy3;
729735
yy19:
730736
++YYCURSOR;
731-
#line 660 "ext/standard/var_unserializer.re"
737+
#line 666 "ext/standard/var_unserializer.re"
732738
{
733739
*p = YYCURSOR;
734740
ZVAL_NULL(rval);
735741
return 1;
736742
}
737-
#line 738 "ext/standard/var_unserializer.c"
743+
#line 744 "ext/standard/var_unserializer.c"
738744
yy21:
739745
yych = *++YYCURSOR;
740746
if (yych <= ',') {
@@ -984,7 +990,7 @@ static int php_var_unserialize_internal(UNSERIALIZE_PARAMETER)
984990
goto yy18;
985991
yy63:
986992
++YYCURSOR;
987-
#line 611 "ext/standard/var_unserializer.re"
993+
#line 617 "ext/standard/var_unserializer.re"
988994
{
989995
zend_long id;
990996

@@ -1009,7 +1015,7 @@ static int php_var_unserialize_internal(UNSERIALIZE_PARAMETER)
10091015

10101016
return 1;
10111017
}
1012-
#line 1013 "ext/standard/var_unserializer.c"
1018+
#line 1019 "ext/standard/var_unserializer.c"
10131019
yy65:
10141020
yych = *++YYCURSOR;
10151021
if (yych == '"') goto yy84;
@@ -1020,13 +1026,13 @@ static int php_var_unserialize_internal(UNSERIALIZE_PARAMETER)
10201026
goto yy18;
10211027
yy67:
10221028
++YYCURSOR;
1023-
#line 666 "ext/standard/var_unserializer.re"
1029+
#line 672 "ext/standard/var_unserializer.re"
10241030
{
10251031
*p = YYCURSOR;
10261032
ZVAL_BOOL(rval, parse_iv(start + 2));
10271033
return 1;
10281034
}
1029-
#line 1030 "ext/standard/var_unserializer.c"
1035+
#line 1036 "ext/standard/var_unserializer.c"
10301036
yy69:
10311037
++YYCURSOR;
10321038
if ((YYLIMIT - YYCURSOR) < 4) YYFILL(4);
@@ -1046,7 +1052,7 @@ static int php_var_unserialize_internal(UNSERIALIZE_PARAMETER)
10461052
}
10471053
yy71:
10481054
++YYCURSOR;
1049-
#line 714 "ext/standard/var_unserializer.re"
1055+
#line 720 "ext/standard/var_unserializer.re"
10501056
{
10511057
#if SIZEOF_ZEND_LONG == 4
10521058
use_double:
@@ -1055,7 +1061,7 @@ static int php_var_unserialize_internal(UNSERIALIZE_PARAMETER)
10551061
ZVAL_DOUBLE(rval, zend_strtod((const char *)start + 2, NULL));
10561062
return 1;
10571063
}
1058-
#line 1059 "ext/standard/var_unserializer.c"
1064+
#line 1065 "ext/standard/var_unserializer.c"
10591065
yy73:
10601066
yych = *++YYCURSOR;
10611067
if (yych <= ',') {
@@ -1077,7 +1083,7 @@ static int php_var_unserialize_internal(UNSERIALIZE_PARAMETER)
10771083
goto yy18;
10781084
yy76:
10791085
++YYCURSOR;
1080-
#line 672 "ext/standard/var_unserializer.re"
1086+
#line 678 "ext/standard/var_unserializer.re"
10811087
{
10821088
#if SIZEOF_ZEND_LONG == 4
10831089
int digits = YYCURSOR - start - 3;
@@ -1103,14 +1109,14 @@ static int php_var_unserialize_internal(UNSERIALIZE_PARAMETER)
11031109
ZVAL_LONG(rval, parse_iv(start + 2));
11041110
return 1;
11051111
}
1106-
#line 1107 "ext/standard/var_unserializer.c"
1112+
#line 1113 "ext/standard/var_unserializer.c"
11071113
yy78:
11081114
yych = *++YYCURSOR;
11091115
if (yych == '"') goto yy92;
11101116
goto yy18;
11111117
yy79:
11121118
++YYCURSOR;
1113-
#line 636 "ext/standard/var_unserializer.re"
1119+
#line 642 "ext/standard/var_unserializer.re"
11141120
{
11151121
zend_long id;
11161122

@@ -1134,14 +1140,14 @@ static int php_var_unserialize_internal(UNSERIALIZE_PARAMETER)
11341140

11351141
return 1;
11361142
}
1137-
#line 1138 "ext/standard/var_unserializer.c"
1143+
#line 1144 "ext/standard/var_unserializer.c"
11381144
yy81:
11391145
yych = *++YYCURSOR;
11401146
if (yych == '"') goto yy94;
11411147
goto yy18;
11421148
yy82:
11431149
++YYCURSOR;
1144-
#line 824 "ext/standard/var_unserializer.re"
1150+
#line 830 "ext/standard/var_unserializer.re"
11451151
{
11461152
size_t len, len2, len3, maxlen;
11471153
zend_long elements;
@@ -1293,10 +1299,10 @@ static int php_var_unserialize_internal(UNSERIALIZE_PARAMETER)
12931299

12941300
return object_common2(UNSERIALIZE_PASSTHRU, elements);
12951301
}
1296-
#line 1297 "ext/standard/var_unserializer.c"
1302+
#line 1303 "ext/standard/var_unserializer.c"
12971303
yy84:
12981304
++YYCURSOR;
1299-
#line 755 "ext/standard/var_unserializer.re"
1305+
#line 761 "ext/standard/var_unserializer.re"
13001306
{
13011307
size_t len, maxlen;
13021308
zend_string *str;
@@ -1330,10 +1336,10 @@ static int php_var_unserialize_internal(UNSERIALIZE_PARAMETER)
13301336
ZVAL_STR(rval, str);
13311337
return 1;
13321338
}
1333-
#line 1334 "ext/standard/var_unserializer.c"
1339+
#line 1340 "ext/standard/var_unserializer.c"
13341340
yy86:
13351341
++YYCURSOR;
1336-
#line 789 "ext/standard/var_unserializer.re"
1342+
#line 795 "ext/standard/var_unserializer.re"
13371343
{
13381344
zend_long elements = parse_iv(start + 2);
13391345
/* use iv() not uiv() in order to check data range */
@@ -1357,7 +1363,7 @@ static int php_var_unserialize_internal(UNSERIALIZE_PARAMETER)
13571363

13581364
return finish_nested_data(UNSERIALIZE_PASSTHRU);
13591365
}
1360-
#line 1361 "ext/standard/var_unserializer.c"
1366+
#line 1367 "ext/standard/var_unserializer.c"
13611367
yy88:
13621368
yych = *++YYCURSOR;
13631369
if (yych <= ',') {
@@ -1382,7 +1388,7 @@ static int php_var_unserialize_internal(UNSERIALIZE_PARAMETER)
13821388
goto yy18;
13831389
yy92:
13841390
++YYCURSOR;
1385-
#line 813 "ext/standard/var_unserializer.re"
1391+
#line 819 "ext/standard/var_unserializer.re"
13861392
{
13871393
zend_long elements;
13881394
if (!var_hash) return 0;
@@ -1393,10 +1399,10 @@ static int php_var_unserialize_internal(UNSERIALIZE_PARAMETER)
13931399
}
13941400
return object_common2(UNSERIALIZE_PASSTHRU, elements);
13951401
}
1396-
#line 1397 "ext/standard/var_unserializer.c"
1402+
#line 1403 "ext/standard/var_unserializer.c"
13971403
yy94:
13981404
++YYCURSOR;
1399-
#line 723 "ext/standard/var_unserializer.re"
1405+
#line 729 "ext/standard/var_unserializer.re"
14001406
{
14011407
size_t len, maxlen;
14021408
char *str;
@@ -1428,15 +1434,15 @@ static int php_var_unserialize_internal(UNSERIALIZE_PARAMETER)
14281434
ZVAL_STRINGL(rval, str, len);
14291435
return 1;
14301436
}
1431-
#line 1432 "ext/standard/var_unserializer.c"
1437+
#line 1438 "ext/standard/var_unserializer.c"
14321438
yy96:
14331439
yych = *++YYCURSOR;
14341440
if (yych <= '/') goto yy18;
14351441
if (yych <= '9') goto yy89;
14361442
goto yy18;
14371443
yy97:
14381444
++YYCURSOR;
1439-
#line 698 "ext/standard/var_unserializer.re"
1445+
#line 704 "ext/standard/var_unserializer.re"
14401446
{
14411447
*p = YYCURSOR;
14421448

@@ -1452,9 +1458,9 @@ static int php_var_unserialize_internal(UNSERIALIZE_PARAMETER)
14521458

14531459
return 1;
14541460
}
1455-
#line 1456 "ext/standard/var_unserializer.c"
1461+
#line 1462 "ext/standard/var_unserializer.c"
14561462
}
1457-
#line 984 "ext/standard/var_unserializer.re"
1463+
#line 990 "ext/standard/var_unserializer.re"
14581464

14591465

14601466
return 0;

‎ext/standard/var_unserializer.re

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -481,16 +481,22 @@ static inline int object_custom(UNSERIALIZE_PARAMETER, zend_class_entry *ce)
481481
return 0;
482482
}
483483

484+
/* Check that '}' is present before calling ce->unserialize() to mitigate issues
485+
* with unserialize reading past the end of the passed buffer if the string is not
486+
* appropriately terminated (usually NUL terminated, but '}' is also sufficient.) */
487+
if ((*p)[datalen] != '}') {
488+
return 0;
489+
}
490+
484491
if (ce->unserialize == NULL) {
485492
zend_error(E_WARNING, "Class %s has no unserializer", ZSTR_VAL(ce->name));
486493
object_init_ex(rval, ce);
487494
} else if (ce->unserialize(rval, ce, (const unsigned char*)*p, datalen, (zend_unserialize_data *)var_hash) != SUCCESS) {
488495
return 0;
489496
}
490497

491-
(*p) += datalen;
492-
493-
return finish_nested_data(UNSERIALIZE_PASSTHRU);
498+
(*p) += datalen + 1; /* +1 for '}' */
499+
return 1;
494500
}
495501

496502
static inline zend_long object_common1(UNSERIALIZE_PARAMETER, zend_class_entry *ce)

0 commit comments

Comments
 (0)