]> granicus.if.org Git - php/commitdiff
Fix #73529: session_decode() silently fails on wrong input
authorChristoph M. Becker <cmbecker69@gmx.de>
Wed, 10 Jun 2020 12:04:51 +0000 (14:04 +0200)
committerChristoph M. Becker <cmbecker69@gmx.de>
Wed, 10 Jun 2020 14:48:49 +0000 (16:48 +0200)
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.

NEWS
ext/session/session.c
ext/session/tests/bug73529.phpt
ext/session/tests/session_decode_error2.phpt
ext/session/tests/session_decode_variation4.phpt
ext/standard/tests/serialize/bug72663_3.phpt
ext/standard/tests/strings/bug72663_2.phpt

diff --git a/NEWS b/NEWS
index 0d872d739d084d87880feda9b926f23d1f9b2cac..231780d31a5dac6f94d57bbc6c19fde23819e113 100644 (file)
--- 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).
index 418a89d8839a3ff689fc2863f6b9df81d006bcc4..69d2e3df3c6e270466360aebc6dce0cdffc3d84b 100644 (file)
@@ -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;
index a14c63bc7f6a707380ed69d77972aaa77076c5fc..7fc4fa6d387bb6a85e6883564eb1dd6212a0883d 100644 (file)
@@ -1,16 +1,15 @@
 --TEST--
 Bug #73529 session_decode() silently fails on wrong input
---XFAIL--
-session_decode() does not return proper status.
 --SKIPIF--
 <?php include('skipif.inc'); ?>
 --FILE--
 <?php
+ob_start();
 
 ini_set("session.serialize_handler", "php_serialize");
 session_start();
 
-$result1 = session_decode(serialize(["foo" => "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) {
index 20415c7a6308da3cacad5f4c4dc253fbdb16b304..5795e0701f9859ad4a37d22385340b569f0c306f 100644 (file)
@@ -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
index f239a86ab991e81da623f7a009590936f24fda8d..1f9d0cbe36807837a9fbc857afd9c66eeb8156af 100644 (file)
@@ -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
index 37d67706f2f1bfd252c96fd8a300146c5ba7c429..706bdf21fb9aba94ebd469a7c8a5774461842be2 100644 (file)
@@ -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) {
 }
index 729694be53fa3736b634112dd3eeb37960e1a49b..fc8978439013ffa51ca3a6ed79114b0ec887a879 100644 (file)
@@ -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