From: Christoph M. Becker Date: Wed, 10 Jun 2020 12:04:51 +0000 (+0200) Subject: Fix #73529: session_decode() silently fails on wrong input X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=08858e7cca223f298e169ed275691769483b2064;p=php 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. --- diff --git a/NEWS b/NEWS index 0d872d739d..231780d31a 100644 --- a/NEWS +++ b/NEWS @@ -149,6 +149,7 @@ PHP NEWS - Session: . Fixed bug #78624 (session_gc return value for user defined session handlers). (bshaffer) + . Fixed bug #73529 (session_decode() silently fails on wrong input). (cmb) - SimpleXML: . Fixed bug #75245 (Don't set content of elements with only whitespaces). diff --git a/ext/session/session.c b/ext/session/session.c index 418a89d883..69d2e3df3c 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 a14c63bc7f..7fc4fa6d38 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 20415c7a63..5795e0701f 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 f239a86ab9..1f9d0cbe36 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 diff --git a/ext/standard/tests/serialize/bug72663_3.phpt b/ext/standard/tests/serialize/bug72663_3.phpt index 37d67706f2..706bdf21fb 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 729694be53..fc89784390 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