Skip to content

Commit 08858e7

Browse files
committed
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.
1 parent 85f5d15 commit 08858e7

File tree

7 files changed

+112
-186
lines changed

7 files changed

+112
-186
lines changed

NEWS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ PHP NEWS
149149
- Session:
150150
. Fixed bug #78624 (session_gc return value for user defined session
151151
handlers). (bshaffer)
152+
. Fixed bug #73529 (session_decode() silently fails on wrong input). (cmb)
152153

153154
- SimpleXML:
154155
. Fixed bug #75245 (Don't set content of elements with only whitespaces).

ext/session/session.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -873,7 +873,7 @@ PS_SERIALIZER_DECODE_FUNC(php_serialize) /* {{{ */
873873
Z_ADDREF_P(&PS(http_session_vars));
874874
zend_hash_update_ind(&EG(symbol_table), var_name, &PS(http_session_vars));
875875
zend_string_release_ex(var_name, 0);
876-
return SUCCESS;
876+
return result || !vallen ? SUCCESS : FAILURE;
877877
}
878878
/* }}} */
879879

@@ -990,7 +990,10 @@ PS_SERIALIZER_DECODE_FUNC(php) /* {{{ */
990990
while (p < endptr) {
991991
q = p;
992992
while (*q != PS_DELIMITER) {
993-
if (++q >= endptr) goto break_outer_loop;
993+
if (++q >= endptr) {
994+
retval = FAILURE;
995+
goto break_outer_loop;
996+
}
994997
}
995998

996999
namelen = q - p;

ext/session/tests/bug73529.phpt

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,15 @@
11
--TEST--
22
Bug #73529 session_decode() silently fails on wrong input
3-
--XFAIL--
4-
session_decode() does not return proper status.
53
--SKIPIF--
64
<?php include('skipif.inc'); ?>
75
--FILE--
86
<?php
7+
ob_start();
98

109
ini_set("session.serialize_handler", "php_serialize");
1110
session_start();
1211

13-
$result1 = session_decode(serialize(["foo" => "bar"]));
12+
$result1 = session_decode('foo|s:3:"bar";');
1413
$session1 = $_SESSION;
1514
session_destroy();
1615

@@ -21,17 +20,24 @@ $result2 = session_decode(serialize(["foo" => "bar"]));
2120
$session2 = $_SESSION;
2221
session_destroy();
2322

23+
echo ob_get_clean();
24+
2425
var_dump($result1);
2526
var_dump($session1);
2627
var_dump($result2);
2728
var_dump($session2);
2829

2930
?>
30-
--EXPECT--
31-
bool(true)
32-
array(1) {
33-
["foo"]=>
34-
string(3) "bar"
31+
--EXPECTF--
32+
Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d
33+
34+
Warning: session_destroy(): Trying to destroy uninitialized session in %s on line %d
35+
36+
Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d
37+
38+
Warning: session_destroy(): Trying to destroy uninitialized session in %s on line %d
39+
bool(false)
40+
array(0) {
3541
}
3642
bool(false)
3743
array(0) {

0 commit comments

Comments
 (0)