Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion ext/session/session.c
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,20 @@ static PHP_INI_MH(OnUpdateSessionStr)
return OnUpdateStr(entry, new_value, mh_arg1, mh_arg2, mh_arg3, stage);
}

static PHP_INI_MH(OnUpdateSessionSameSite)
{
SESSION_CHECK_ACTIVE_STATE;
SESSION_CHECK_OUTPUT_STATE;

if (new_value && ZSTR_LEN(new_value) > 0 && !php_is_valid_samesite_value(new_value)) {
php_error_docref(NULL, E_WARNING,
"session.cookie_samesite must be \"Strict\", \"Lax\", \"None\", or \"\"");
return FAILURE;
}

return OnUpdateStr(entry, new_value, mh_arg1, mh_arg2, mh_arg3, stage);
}

static PHP_INI_MH(OnUpdateSessionBool)
{
SESSION_CHECK_ACTIVE_STATE;
Expand Down Expand Up @@ -904,7 +918,7 @@ PHP_INI_BEGIN()
STD_PHP_INI_BOOLEAN("session.cookie_secure", "0", PHP_INI_ALL, OnUpdateSessionBool, cookie_secure, php_ps_globals, ps_globals)
STD_PHP_INI_BOOLEAN("session.cookie_partitioned", "0", PHP_INI_ALL, OnUpdateSessionBool, cookie_partitioned, php_ps_globals, ps_globals)
STD_PHP_INI_BOOLEAN("session.cookie_httponly", "0", PHP_INI_ALL, OnUpdateSessionBool, cookie_httponly, php_ps_globals, ps_globals)
STD_PHP_INI_ENTRY("session.cookie_samesite", "", PHP_INI_ALL, OnUpdateSessionStr, cookie_samesite, php_ps_globals, ps_globals)
STD_PHP_INI_ENTRY("session.cookie_samesite", "", PHP_INI_ALL, OnUpdateSessionSameSite, cookie_samesite, php_ps_globals, ps_globals)
STD_PHP_INI_BOOLEAN("session.use_cookies", "1", PHP_INI_ALL, OnUpdateSessionBool, use_cookies, php_ps_globals, ps_globals)
STD_PHP_INI_BOOLEAN("session.use_only_cookies", "1", PHP_INI_ALL, OnUpdateUseOnlyCookies, use_only_cookies, php_ps_globals, ps_globals)
STD_PHP_INI_BOOLEAN("session.use_strict_mode", "0", PHP_INI_ALL, OnUpdateSessionBool, use_strict_mode, php_ps_globals, ps_globals)
Expand Down
6 changes: 3 additions & 3 deletions ext/session/tests/session_get_cookie_params_basic.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ var_dump(session_set_cookie_params([
"domain" => "baz",
"secure" => FALSE,
"httponly" => FALSE,
"samesite" => "please"]));
"samesite" => "Strict"]));
var_dump(session_get_cookie_params());
var_dump(session_set_cookie_params([
"secure" => TRUE,
Expand Down Expand Up @@ -107,7 +107,7 @@ array(7) {
["httponly"]=>
bool(false)
["samesite"]=>
string(6) "please"
string(6) "Strict"
}
bool(true)
array(7) {
Expand All @@ -124,6 +124,6 @@ array(7) {
["httponly"]=>
bool(false)
["samesite"]=>
string(6) "please"
string(6) "Strict"
}
Done
6 changes: 3 additions & 3 deletions ext/session/tests/session_get_cookie_params_variation1.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ ini_set("session.cookie_secure", TRUE);
var_dump(session_get_cookie_params());
ini_set("session.cookie_httponly", TRUE);
var_dump(session_get_cookie_params());
ini_set("session.cookie_samesite", "foo");
ini_set("session.cookie_samesite", "Lax");
var_dump(session_get_cookie_params());
ini_set("session.cookie_partitioned", TRUE);
var_dump(session_get_cookie_params());
Expand Down Expand Up @@ -150,7 +150,7 @@ array(7) {
["httponly"]=>
bool(true)
["samesite"]=>
string(3) "foo"
string(3) "Lax"
}
array(7) {
["lifetime"]=>
Expand All @@ -166,6 +166,6 @@ array(7) {
["httponly"]=>
bool(true)
["samesite"]=>
string(3) "foo"
string(3) "Lax"
}
Done
22 changes: 22 additions & 0 deletions ext/session/tests/session_set_cookie_params_invalid_ini.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
--TEST--
Test session.cookie_samesite with invalid INI value
--INI--
session.cookie_samesite=Invalid
--EXTENSIONS--
session
--SKIPIF--
<?php include('skipif.inc'); ?>
--FILE--
<?php

ob_start();

var_dump(ini_get("session.cookie_samesite"));

echo "Done";
ob_end_flush();
?>
--EXPECTF--
Warning: PHP Startup: session.cookie_samesite must be "Strict", "Lax", "None", or "" in Unknown on line 0
string(0) ""
Done
54 changes: 37 additions & 17 deletions ext/session/tests/session_set_cookie_params_variation6.phpt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
--TEST--
Test session_set_cookie_params() function : variation
Test session_set_cookie_params() samesite validation
--INI--
session.cookie_samesite=test
session.cookie_samesite=Lax
--EXTENSIONS--
session
--SKIPIF--
Expand All @@ -11,36 +11,56 @@ session

ob_start();

echo "*** Testing session_set_cookie_params() : variation ***\n";

echo "-- Valid values --\n";
var_dump(ini_get("session.cookie_samesite"));
var_dump(session_set_cookie_params(["samesite" => "nothing"]));
var_dump(session_set_cookie_params(["samesite" => "Strict"]));
var_dump(ini_get("session.cookie_samesite"));
var_dump(session_start());
var_dump(session_set_cookie_params(["samesite" => "None"]));
var_dump(ini_get("session.cookie_samesite"));
var_dump(session_set_cookie_params(["samesite" => "test"]));
var_dump(session_set_cookie_params(["samesite" => ""]));
var_dump(ini_get("session.cookie_samesite"));
var_dump(session_destroy());

echo "-- Invalid value via session_set_cookie_params --\n";
var_dump(session_set_cookie_params(["samesite" => "Invalid"]));
var_dump(ini_get("session.cookie_samesite"));
var_dump(session_set_cookie_params(["samesite" => "other"]));

echo "-- Invalid value via ini_set --\n";
var_dump(ini_set("session.cookie_samesite", "Invalid"));
var_dump(ini_get("session.cookie_samesite"));

echo "-- Cannot change while session is active --\n";
var_dump(session_set_cookie_params(["samesite" => "Lax"]));
var_dump(session_start());
var_dump(session_set_cookie_params(["samesite" => "Strict"]));
var_dump(session_destroy());

echo "Done";
ob_end_flush();
?>
--EXPECTF--
*** Testing session_set_cookie_params() : variation ***
string(4) "test"
-- Valid values --
string(3) "Lax"
bool(true)
string(7) "nothing"
string(6) "Strict"
bool(true)
string(7) "nothing"
string(4) "None"
bool(true)
string(0) ""
-- Invalid value via session_set_cookie_params --

Warning: session_set_cookie_params(): Session cookie parameters cannot be changed when a session is active (started from %s on line %d) in %s on line %d
Warning: session_set_cookie_params(): session.cookie_samesite must be "Strict", "Lax", "None", or "" in %s on line %d
bool(false)
string(0) ""
-- Invalid value via ini_set --

Warning: ini_set(): session.cookie_samesite must be "Strict", "Lax", "None", or "" in %s on line %d
bool(false)
string(7) "nothing"
string(0) ""
-- Cannot change while session is active --
bool(true)
string(7) "nothing"
bool(true)
string(5) "other"

Warning: session_set_cookie_params(): Session cookie parameters cannot be changed when a session is active (started from %s on line %d) in %s on line %d
bool(false)
bool(true)
Done
4 changes: 2 additions & 2 deletions ext/session/tests/session_set_cookie_params_variation7.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ try {

var_dump(ini_get("session.cookie_secure"));
var_dump(ini_get("session.cookie_samesite"));
var_dump(session_set_cookie_params(["secure" => true, "samesite" => "please"]));
var_dump(session_set_cookie_params(["secure" => true, "samesite" => "Strict"]));
var_dump(ini_get("session.cookie_secure"));
var_dump(ini_get("session.cookie_samesite"));

Expand Down Expand Up @@ -66,7 +66,7 @@ string(1) "0"
string(0) ""
bool(true)
string(1) "1"
string(6) "please"
string(6) "Strict"
string(1) "0"
bool(true)
string(2) "42"
Expand Down
13 changes: 12 additions & 1 deletion ext/standard/head.c
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,13 @@ PHPAPI bool php_header(void)
}
}

PHPAPI bool php_is_valid_samesite_value(zend_string *value)
{
return zend_string_equals_literal_ci(value, "Strict")
|| zend_string_equals_literal_ci(value, "Lax")
|| zend_string_equals_literal_ci(value, "None");
}

#define ILLEGAL_COOKIE_CHARACTER "\",\", \";\", \" \", \"\\t\", \"\\r\", \"\\n\", \"\\013\", or \"\\014\""
PHPAPI zend_result php_setcookie(zend_string *name, zend_string *value, time_t expires,
zend_string *path, zend_string *domain, bool secure, bool httponly,
Expand Down Expand Up @@ -123,7 +130,11 @@ PHPAPI zend_result php_setcookie(zend_string *name, zend_string *value, time_t e
return FAILURE;
}

/* Should check value of SameSite? */
if (samesite && ZSTR_LEN(samesite) > 0 && !php_is_valid_samesite_value(samesite)) {
zend_value_error("%s(): \"samesite\" option must be \"Strict\", \"Lax\", \"None\", or \"\"",
get_active_function_name());
return FAILURE;
}

if (value == NULL || ZSTR_LEN(value) == 0) {
/*
Expand Down
2 changes: 2 additions & 0 deletions ext/standard/head.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
#define COOKIE_SAMESITE "; SameSite="
#define COOKIE_PARTITIONED "; Partitioned"

PHPAPI bool php_is_valid_samesite_value(zend_string *value);

extern PHP_RINIT_FUNCTION(head);

PHPAPI bool php_header(void);
Expand Down
52 changes: 52 additions & 0 deletions ext/standard/tests/setcookie_samesite_validation.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
--TEST--
setcookie() and setrawcookie() validate samesite option
--FILE--
<?php
ob_start();

// Valid values
var_dump(setcookie('test', 'value', ['samesite' => 'Strict']));
var_dump(setcookie('test', 'value', ['samesite' => 'Lax']));
var_dump(setcookie('test', 'value', ['samesite' => 'None']));
var_dump(setcookie('test', 'value', ['samesite' => '']));

// Case-insensitive
var_dump(setcookie('test', 'value', ['samesite' => 'strict']));
var_dump(setcookie('test', 'value', ['samesite' => 'LAX']));
var_dump(setcookie('test', 'value', ['samesite' => 'NONE']));

// setrawcookie uses the same validation
var_dump(setrawcookie('test', 'value', ['samesite' => 'Lax']));

// Invalid values
try {
setcookie('test', 'value', ['samesite' => 'Invalid']);
} catch (ValueError $e) {
echo $e->getMessage() . "\n";
}

try {
setcookie('test', 'value', ['samesite' => "Strict\r\nX-Injected: evil"]);
} catch (ValueError $e) {
echo $e->getMessage() . "\n";
}

try {
setrawcookie('test', 'value', ['samesite' => 'Invalid']);
} catch (ValueError $e) {
echo $e->getMessage() . "\n";
}

?>
--EXPECTF--
bool(true)
bool(true)
bool(true)
bool(true)
bool(true)
bool(true)
bool(true)
bool(true)
setcookie(): "samesite" option must be "Strict", "Lax", "None", or ""
setcookie(): "samesite" option must be "Strict", "Lax", "None", or ""
setrawcookie(): "samesite" option must be "Strict", "Lax", "None", or ""
Loading