diff --git a/NEWS b/NEWS index 44631c9b610ff..d1f9267b44149 100644 --- a/NEWS +++ b/NEWS @@ -38,6 +38,8 @@ PHP NEWS - Standard: . Fix GH-10187 (Segfault in stripslashes() with arm64). (nielsdos) + . Fixed bug GH-10214 (Incomplete validation of object syntax during + unserialize()). (timwolla) 05 Jan 2023, PHP 8.2.1 diff --git a/UPGRADING b/UPGRADING index 5f69712a4b02b..bc92affa46e80 100644 --- a/UPGRADING +++ b/UPGRADING @@ -223,6 +223,10 @@ PHP 8.2 UPGRADE NOTES widened to iterable from Iterator, allowing arrays to be passed. RFC: https://wiki.php.net/rfc/iterator_xyz_accept_array +- Standard + . unserialize() now performs a stricter validation of the structure of serialized + objects. + - XML . xml_parser_set_option() now actually returns false when attempting to set a negative tag start. Previously it returned true while emitting an E_WARNING. diff --git a/ext/gmp/tests/bug74670.phpt b/ext/gmp/tests/bug74670.phpt index 5f9de9330e591..1d09509a91316 100644 --- a/ext/gmp/tests/bug74670.phpt +++ b/ext/gmp/tests/bug74670.phpt @@ -8,5 +8,5 @@ $str = 'C:3:"GMP":4:{s:6666666666:""}'; var_dump(unserialize($str)); ?> --EXPECTF-- -Notice: unserialize(): Error at offset 13 of 29 bytes in %s on line %d +Notice: unserialize(): Error at offset 17 of 29 bytes in %s on line %d bool(false) diff --git a/ext/spl/tests/bug73029.phpt b/ext/spl/tests/bug73029.phpt index 5c3cd8420da8d..9e002f1182939 100644 --- a/ext/spl/tests/bug73029.phpt +++ b/ext/spl/tests/bug73029.phpt @@ -3,6 +3,13 @@ Bug #73029: Missing type check when unserializing SplArray --FILE-- getMessage() . "\n"; +} +try { $a = 'C:11:"ArrayObject":19:0x:i:0;r:2;;m:a:0:{}}'; $m = unserialize($a); $x = $m[2]; @@ -11,6 +18,10 @@ $x = $m[2]; } ?> DONE ---EXPECT-- +--EXPECTF-- Error at offset 10 of 19 bytes + +Notice: unserialize(): Error at offset 22 of 43 bytes in %s on line %d + +Warning: Trying to access array offset on value of type bool in %s on line %d DONE diff --git a/ext/standard/tests/serialize/bug73341.phpt b/ext/standard/tests/serialize/bug73341.phpt index 2f38949c1ae8f..3bdad06440be0 100644 --- a/ext/standard/tests/serialize/bug73341.phpt +++ b/ext/standard/tests/serialize/bug73341.phpt @@ -2,6 +2,13 @@ Bug #73144 (Use-afte-free in ArrayObject Deserialization) --FILE-- getMessage()."\n"; +} + try { $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}'; $obj = unserialize($token); @@ -20,5 +27,7 @@ unserialize($exploit); --EXPECTF-- Error at offset 6 of 7 bytes +Notice: unserialize(): Error at offset 19 of 79 bytes in %s on line %d + Notice: ArrayObject::unserialize(): Unexpected end of serialized data in %sbug73341.php on line %d Error at offset 24 of 34 bytes diff --git a/ext/standard/tests/serialize/bug74111.phpt b/ext/standard/tests/serialize/bug74111.phpt index 62922bea55ae0..2062687aa271e 100644 --- a/ext/standard/tests/serialize/bug74111.phpt +++ b/ext/standard/tests/serialize/bug74111.phpt @@ -6,5 +6,5 @@ $s = 'O:8:"stdClass":00000000'; var_dump(unserialize($s)); ?> --EXPECTF-- -Notice: unserialize(): Error at offset 25 of 23 bytes in %s on line %d +Notice: unserialize(): Error at offset 23 of 23 bytes in %s on line %d bool(false) diff --git a/ext/standard/tests/serialize/serialization_objects_017.phpt b/ext/standard/tests/serialize/serialization_objects_017.phpt new file mode 100644 index 0000000000000..901d73f2b24aa --- /dev/null +++ b/ext/standard/tests/serialize/serialization_objects_017.phpt @@ -0,0 +1,17 @@ +--TEST-- +Object serialization / unserialization: Strict format +--FILE-- + +--EXPECTF-- +Notice: unserialize(): Error at offset 9 of 22 bytes in %s on line %d +bool(false) + +Notice: unserialize(): Error at offset 10 of 22 bytes in %s on line %d +bool(false) diff --git a/ext/standard/tests/serialize/serialization_objects_018.phpt b/ext/standard/tests/serialize/serialization_objects_018.phpt new file mode 100644 index 0000000000000..e05ff5ca04608 --- /dev/null +++ b/ext/standard/tests/serialize/serialization_objects_018.phpt @@ -0,0 +1,33 @@ +--TEST-- +Object serialization / unserialization: Strict format (2) +--FILE-- + +--EXPECTF-- +Notice: unserialize(): Error at offset 9 of 15 bytes in %s on line %d +bool(false) + +Notice: unserialize(): Error at offset 10 of 15 bytes in %s on line %d +bool(false) + +Notice: unserialize(): Error at offset 14 of 15 bytes in %s on line %d +bool(false) + +Notice: unserialize(): Error at offset 8 of 8 bytes in %s on line %d +bool(false) diff --git a/ext/standard/var_unserializer.re b/ext/standard/var_unserializer.re index bcf19002fc0a2..313d7fadeb394 100644 --- a/ext/standard/var_unserializer.re +++ b/ext/standard/var_unserializer.re @@ -743,6 +743,19 @@ static inline int object_custom(UNSERIALIZE_PARAMETER, zend_class_entry *ce) datalen = parse_iv2((*p) + 2, p); + if (max - (*p) < 2) { + return 0; + } + + if ((*p)[0] != ':') { + return 0; + } + + if ((*p)[1] != '{') { + (*p) += 1; + return 0; + } + (*p) += 2; if (datalen < 0 || (max - (*p)) <= datalen) { @@ -754,6 +767,7 @@ static inline int object_custom(UNSERIALIZE_PARAMETER, zend_class_entry *ce) * with unserialize reading past the end of the passed buffer if the string is not * appropriately terminated (usually NUL terminated, but '}' is also sufficient.) */ if ((*p)[datalen] != '}') { + (*p) += datalen; return 0; } @@ -1293,6 +1307,16 @@ object ":" uiv ":" ["] { return 0; } + YYCURSOR = *p; + + if (*(YYCURSOR) != ':') { + return 0; + } + if (*(YYCURSOR+1) != '{') { + *p = YYCURSOR+1; + return 0; + } + *p += 2; has_unserialize = !incomplete_class && ce->__unserialize;