Skip to content

unserialize: Strictly check for :{ at object start #10214

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jan 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 4 additions & 0 deletions UPGRADING
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion ext/gmp/tests/bug74670.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
13 changes: 12 additions & 1 deletion ext/spl/tests/bug73029.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,13 @@ Bug #73029: Missing type check when unserializing SplArray
--FILE--
<?php
try {
$a = 'C:11:"ArrayObject":19:{x:i:0;r:2;;m:a:0:{}}';
$m = unserialize($a);
$x = $m[2];
} catch(UnexpectedValueException $e) {
print $e->getMessage() . "\n";
}
try {
$a = 'C:11:"ArrayObject":19:0x:i:0;r:2;;m:a:0:{}}';
$m = unserialize($a);
$x = $m[2];
Expand All @@ -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
9 changes: 9 additions & 0 deletions ext/standard/tests/serialize/bug73341.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@
Bug #73144 (Use-afte-free in ArrayObject Deserialization)
--FILE--
<?php
try {
$token = 'a:2:{i:0;O:1:"0":2:{s:1:"0";i:0;s:1:"0";a:1:{i:0;C:11:"ArrayObject":7:{x:i:0;r}';
$obj = unserialize($token);
} catch(Exception $e) {
echo $e->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);
Expand All @@ -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
2 changes: 1 addition & 1 deletion ext/standard/tests/serialize/bug74111.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
17 changes: 17 additions & 0 deletions ext/standard/tests/serialize/serialization_objects_017.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
--TEST--
Object serialization / unserialization: Strict format
--FILE--
<?php
class A {public $a;}

var_dump(unserialize('O:1:"A":1x{s:1:"a";N;}'));
//0123456789012345678901
var_dump(unserialize('O:1:"A":1:xs:1:"a";N;}'));
//0123456789012345678901
?>
--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)
33 changes: 33 additions & 0 deletions ext/standard/tests/serialize/serialization_objects_018.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
--TEST--
Object serialization / unserialization: Strict format (2)
--FILE--
<?php
class A implements Serializable {
public function serialize() {}
public function unserialize($data) {}
public function __serialize() {}
public function __unserialize($data) {}
}

var_dump(unserialize('C:1:"A":3x{foo}'));
//012345678901234
var_dump(unserialize('C:1:"A":3:xfoo}'));
//012345678901234
var_dump(unserialize('C:1:"A":3:{foox'));
//012345678901234
var_dump(unserialize('C:1:"A":'));
//01234567

?>
--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)
24 changes: 24 additions & 0 deletions ext/standard/var_unserializer.re
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down