From 96250e93383f95bc693573088f39ff0fa29db684 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Wed, 10 Jun 2020 11:59:38 +0200 Subject: [PATCH 1/2] Fix #73529: session_decode() silently fails on wrong input The `php_serialize` decode function has to return `FAILURE`, if the unserialization failed on anything but an empty string. The `php` decode function has also to return `FAILURE`, if there is trailing garbage in the string. --- ext/session/session.c | 7 +- ext/session/tests/bug73529.phpt | 22 +- ext/session/tests/session_decode_error2.phpt | 246 ++++++------------ .../tests/session_decode_variation4.phpt | 18 +- 4 files changed, 107 insertions(+), 186 deletions(-) diff --git a/ext/session/session.c b/ext/session/session.c index 418a89d8839a3..69d2e3df3c6e2 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -873,7 +873,7 @@ PS_SERIALIZER_DECODE_FUNC(php_serialize) /* {{{ */ Z_ADDREF_P(&PS(http_session_vars)); zend_hash_update_ind(&EG(symbol_table), var_name, &PS(http_session_vars)); zend_string_release_ex(var_name, 0); - return SUCCESS; + return result || !vallen ? SUCCESS : FAILURE; } /* }}} */ @@ -990,7 +990,10 @@ PS_SERIALIZER_DECODE_FUNC(php) /* {{{ */ while (p < endptr) { q = p; while (*q != PS_DELIMITER) { - if (++q >= endptr) goto break_outer_loop; + if (++q >= endptr) { + retval = FAILURE; + goto break_outer_loop; + } } namelen = q - p; diff --git a/ext/session/tests/bug73529.phpt b/ext/session/tests/bug73529.phpt index a14c63bc7f6a7..7fc4fa6d387bb 100644 --- a/ext/session/tests/bug73529.phpt +++ b/ext/session/tests/bug73529.phpt @@ -1,16 +1,15 @@ --TEST-- Bug #73529 session_decode() silently fails on wrong input ---XFAIL-- -session_decode() does not return proper status. --SKIPIF-- --FILE-- "bar"])); +$result1 = session_decode('foo|s:3:"bar";'); $session1 = $_SESSION; session_destroy(); @@ -21,17 +20,24 @@ $result2 = session_decode(serialize(["foo" => "bar"])); $session2 = $_SESSION; session_destroy(); +echo ob_get_clean(); + var_dump($result1); var_dump($session1); var_dump($result2); var_dump($session2); ?> ---EXPECT-- -bool(true) -array(1) { - ["foo"]=> - string(3) "bar" +--EXPECTF-- +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d + +Warning: session_destroy(): Trying to destroy uninitialized session in %s on line %d + +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d + +Warning: session_destroy(): Trying to destroy uninitialized session in %s on line %d +bool(false) +array(0) { } bool(false) array(0) { diff --git a/ext/session/tests/session_decode_error2.phpt b/ext/session/tests/session_decode_error2.phpt index 20415c7a6308d..5795e0701f985 100644 --- a/ext/session/tests/session_decode_error2.phpt +++ b/ext/session/tests/session_decode_error2.phpt @@ -39,226 +39,232 @@ array(0) { } -- Iteration 1 -- -bool(true) + +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d +bool(false) array(0) { } -- Iteration 2 -- -bool(true) + +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d +bool(false) array(0) { } -- Iteration 3 -- -bool(true) + +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d +bool(false) array(0) { } -- Iteration 4 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 5 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 6 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 7 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 8 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 9 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 10 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 11 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 12 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 13 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 14 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 15 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 16 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 17 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 18 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 19 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 20 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 21 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 22 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 23 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 24 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 25 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 26 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 27 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 28 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 29 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 30 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 31 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 32 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 33 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } @@ -278,85 +284,57 @@ array(1) { } -- Iteration 35 -- -bool(true) -array(1) { - ["foo"]=> - array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } + +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d +bool(false) +array(0) { } -- Iteration 36 -- -bool(true) -array(1) { - ["foo"]=> - array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } + +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d +bool(false) +array(0) { } -- Iteration 37 -- -bool(true) -array(1) { - ["foo"]=> - array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } + +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d +bool(false) +array(0) { } -- Iteration 38 -- -bool(true) -array(1) { - ["foo"]=> - array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } + +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d +bool(false) +array(0) { } -- Iteration 39 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 40 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 41 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 42 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } @@ -385,125 +363,61 @@ array(2) { } -- Iteration 44 -- -bool(true) -array(2) { - ["foo"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } - ["guff"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } + +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d +bool(false) +array(0) { } -- Iteration 45 -- -bool(true) -array(2) { - ["foo"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } - ["guff"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } + +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d +bool(false) +array(0) { } -- Iteration 46 -- -bool(true) -array(2) { - ["foo"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } - ["guff"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } + +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d +bool(false) +array(0) { } -- Iteration 47 -- -bool(true) -array(2) { - ["foo"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } - ["guff"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } + +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d +bool(false) +array(0) { } -- Iteration 48 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 49 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 50 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -- Iteration 51 -- -Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s%esession_decode_error2.php on line %d +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d bool(false) array(0) { } -Warning: session_destroy(): Trying to destroy uninitialized session in %s%esession_decode_error2.php on line %d +Warning: session_destroy(): Trying to destroy uninitialized session in %s on line %d bool(false) Done diff --git a/ext/session/tests/session_decode_variation4.phpt b/ext/session/tests/session_decode_variation4.phpt index f239a86ab991e..1f9d0cbe36807 100644 --- a/ext/session/tests/session_decode_variation4.phpt +++ b/ext/session/tests/session_decode_variation4.phpt @@ -29,7 +29,7 @@ var_dump(session_destroy()); echo "Done"; ob_end_flush(); ?> ---EXPECT-- +--EXPECTF-- *** Testing session_decode() : variation *** bool(true) array(0) { @@ -42,14 +42,12 @@ array(3) { ["guff"]=> float(123.456) } -bool(true) -array(3) { - ["foo"]=> - int(1234567890) - ["bar"]=> - string(5) "Blah!" - ["guff"]=> - float(123.456) + +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d +bool(false) +array(0) { } -bool(true) + +Warning: session_destroy(): Trying to destroy uninitialized session in %s on line %d +bool(false) Done From f22abcde5f4a21e661c129782fd18b2fb26df230 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Wed, 10 Jun 2020 14:04:51 +0200 Subject: [PATCH 2/2] Fix failing tests --- ext/standard/tests/serialize/bug72663_3.phpt | 2 ++ ext/standard/tests/strings/bug72663_2.phpt | 2 ++ 2 files changed, 4 insertions(+) diff --git a/ext/standard/tests/serialize/bug72663_3.phpt b/ext/standard/tests/serialize/bug72663_3.phpt index 37d67706f2f1b..706bdf21fb9ab 100644 --- a/ext/standard/tests/serialize/bug72663_3.phpt +++ b/ext/standard/tests/serialize/bug72663_3.phpt @@ -13,5 +13,7 @@ var_dump($_SESSION); ?> --EXPECTF-- Notice: session_decode(): Unexpected end of serialized data in %s on line %d + +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d array(0) { } diff --git a/ext/standard/tests/strings/bug72663_2.phpt b/ext/standard/tests/strings/bug72663_2.phpt index 729694be53fa3..fc8978439013f 100644 --- a/ext/standard/tests/strings/bug72663_2.phpt +++ b/ext/standard/tests/strings/bug72663_2.phpt @@ -18,6 +18,8 @@ var_dump($_SESSION); DONE --EXPECTF-- Notice: session_decode(): Unexpected end of serialized data in %sbug72663_2.php on line %d + +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d array(0) { } DONE