Skip to content

Commit c76b831

Browse files
committed
[ext/session] Validate session.cookie_samesite against allowed values
Add OnUpdateSessionSameSite() INI handler that enforces the SameSite whitelist (Strict, Lax, None, or empty string), rejecting any other value with E_WARNING. Previously OnUpdateSessionStr() accepted arbitrary strings including CRLF sequences, which were appended verbatim into the Set-Cookie header. The php_setcookie() function has a TODO comment for the same gap; this addresses it at the INI layer.
1 parent 5e45c17 commit c76b831

File tree

2 files changed

+49
-1
lines changed

2 files changed

+49
-1
lines changed

ext/session/session.c

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -733,6 +733,23 @@ static PHP_INI_MH(OnUpdateSessionStr)
733733
return OnUpdateStr(entry, new_value, mh_arg1, mh_arg2, mh_arg3, stage);
734734
}
735735

736+
static PHP_INI_MH(OnUpdateSessionSameSite)
737+
{
738+
SESSION_CHECK_ACTIVE_STATE;
739+
SESSION_CHECK_OUTPUT_STATE;
740+
741+
if (new_value && ZSTR_LEN(new_value) > 0
742+
&& !zend_string_equals_literal_ci(new_value, "Strict")
743+
&& !zend_string_equals_literal_ci(new_value, "Lax")
744+
&& !zend_string_equals_literal_ci(new_value, "None")) {
745+
php_error_docref(NULL, E_WARNING,
746+
"session.cookie_samesite must be \"Strict\", \"Lax\", \"None\", or \"\"");
747+
return FAILURE;
748+
}
749+
750+
return OnUpdateStr(entry, new_value, mh_arg1, mh_arg2, mh_arg3, stage);
751+
}
752+
736753
static PHP_INI_MH(OnUpdateSessionBool)
737754
{
738755
SESSION_CHECK_ACTIVE_STATE;
@@ -904,7 +921,7 @@ PHP_INI_BEGIN()
904921
STD_PHP_INI_BOOLEAN("session.cookie_secure", "0", PHP_INI_ALL, OnUpdateSessionBool, cookie_secure, php_ps_globals, ps_globals)
905922
STD_PHP_INI_BOOLEAN("session.cookie_partitioned", "0", PHP_INI_ALL, OnUpdateSessionBool, cookie_partitioned, php_ps_globals, ps_globals)
906923
STD_PHP_INI_BOOLEAN("session.cookie_httponly", "0", PHP_INI_ALL, OnUpdateSessionBool, cookie_httponly, php_ps_globals, ps_globals)
907-
STD_PHP_INI_ENTRY("session.cookie_samesite", "", PHP_INI_ALL, OnUpdateSessionStr, cookie_samesite, php_ps_globals, ps_globals)
924+
STD_PHP_INI_ENTRY("session.cookie_samesite", "", PHP_INI_ALL, OnUpdateSessionSameSite, cookie_samesite, php_ps_globals, ps_globals)
908925
STD_PHP_INI_BOOLEAN("session.use_cookies", "1", PHP_INI_ALL, OnUpdateSessionBool, use_cookies, php_ps_globals, ps_globals)
909926
STD_PHP_INI_BOOLEAN("session.use_only_cookies", "1", PHP_INI_ALL, OnUpdateUseOnlyCookies, use_only_cookies, php_ps_globals, ps_globals)
910927
STD_PHP_INI_BOOLEAN("session.use_strict_mode", "0", PHP_INI_ALL, OnUpdateSessionBool, use_strict_mode, php_ps_globals, ps_globals)
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
--TEST--
2+
session.cookie_samesite only accepts Strict, Lax, None, or empty string
3+
--EXTENSIONS--
4+
session
5+
--SKIPIF--
6+
<?php include('skipif.inc'); ?>
7+
--FILE--
8+
<?php
9+
ob_start();
10+
ini_set('session.cookie_samesite', 'Invalid');
11+
ini_set('session.cookie_samesite', "Strict\r\nX-Injected: evil");
12+
$after_invalid = ini_get('session.cookie_samesite');
13+
14+
$r_strict = ini_set('session.cookie_samesite', 'Strict') !== false;
15+
$r_lax = ini_set('session.cookie_samesite', 'Lax') !== false;
16+
$r_none = ini_set('session.cookie_samesite', 'None') !== false;
17+
$r_empty = ini_set('session.cookie_samesite', '') !== false;
18+
echo ob_get_clean();
19+
20+
var_dump($after_invalid);
21+
var_dump($r_strict, $r_lax, $r_none, $r_empty);
22+
?>
23+
--EXPECTF--
24+
Warning: ini_set(): session.cookie_samesite must be "Strict", "Lax", "None", or "" in %s on line %d
25+
26+
Warning: ini_set(): session.cookie_samesite must be "Strict", "Lax", "None", or "" in %s on line %d
27+
string(0) ""
28+
bool(true)
29+
bool(true)
30+
bool(true)
31+
bool(true)

0 commit comments

Comments
 (0)