Skip to content

Warning to error promotion for set(raw)cookie() #5819

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
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
161 changes: 61 additions & 100 deletions ext/standard/head.c
Original file line number Diff line number Diff line change
Expand Up @@ -76,36 +76,41 @@ PHPAPI int php_header(void)
}
}

PHPAPI int php_setcookie(zend_string *name, zend_string *value, time_t expires, zend_string *path, zend_string *domain, int secure, int httponly, zend_string *samesite, int url_encode)
#define ILLEGAL_COOKIE_CHARACTER "\",\", \";\", \" \", \"\\t\", \"\\r\", \"\\n\", \"\\013\", and \"\\014\""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#define ILLEGAL_COOKIE_CHARACTER "\",\", \";\", \" \", \"\\t\", \"\\r\", \"\\n\", \"\\013\", and \"\\014\""
#define ILLEGAL_COOKIE_CHARACTER "\",\", \";\", \" \", \"\\t\", \"\\r\", \"\\n\", \"\\013\", or \"\\014\""

I think...

PHPAPI zend_result php_setcookie(zend_string *name, zend_string *value, time_t expires,
zend_string *path, zend_string *domain, bool secure, bool httponly,
zend_string *samesite, bool url_encode)
{
zend_string *dt;
sapi_header_line ctr = {0};
int result;
zend_result result;
smart_str buf = {0};

if (!ZSTR_LEN(name)) {
zend_error( E_WARNING, "Cookie names must not be empty" );
zend_argument_value_error(1, "cannot be empty");
return FAILURE;
} else if (strpbrk(ZSTR_VAL(name), "=,; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
zend_error(E_WARNING, "Cookie names cannot contain any of the following '=,; \\t\\r\\n\\013\\014'" );
}
if (strpbrk(ZSTR_VAL(name), "=,; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
zend_argument_value_error(1, "cannot contain \"=\", " ILLEGAL_COOKIE_CHARACTER);
return FAILURE;
}

if (!url_encode && value &&
strpbrk(ZSTR_VAL(value), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
zend_error(E_WARNING, "Cookie values cannot contain any of the following ',; \\t\\r\\n\\013\\014'" );
zend_argument_value_error(2, "cannot contain " ILLEGAL_COOKIE_CHARACTER);
return FAILURE;
}

if (path && strpbrk(ZSTR_VAL(path), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
zend_error(E_WARNING, "Cookie paths cannot contain any of the following ',; \\t\\r\\n\\013\\014'" );
zend_value_error("%s(): \"path\" option cannot contain " ILLEGAL_COOKIE_CHARACTER,
get_active_function_name());
return FAILURE;
}

if (domain && strpbrk(ZSTR_VAL(domain), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
zend_error(E_WARNING, "Cookie domains cannot contain any of the following ',; \\t\\r\\n\\013\\014'" );
zend_value_error("%s(): \"domain\" option cannot contain " ILLEGAL_COOKIE_CHARACTER,
get_active_function_name());
return FAILURE;
}
/* Should check value of SameSite? */

if (value == NULL || ZSTR_LEN(value) == 0) {
/*
Expand Down Expand Up @@ -142,7 +147,8 @@ PHPAPI int php_setcookie(zend_string *name, zend_string *value, time_t expires,
if (!p || *(p + 5) != ' ') {
zend_string_free(dt);
smart_str_free(&buf);
zend_error(E_WARNING, "Expiry date cannot have a year greater than 9999");
zend_value_error("%s(): \"expires\" option cannot have a year greater than 9999",
get_active_function_name());
return FAILURE;
}

Expand Down Expand Up @@ -186,49 +192,40 @@ PHPAPI int php_setcookie(zend_string *name, zend_string *value, time_t expires,
return result;
}

static void php_head_parse_cookie_options_array(zval *options, zend_long *expires, zend_string **path, zend_string **domain, zend_bool *secure, zend_bool *httponly, zend_string **samesite) {
int found = 0;
static zend_result php_head_parse_cookie_options_array(zval *options, zend_long *expires, zend_string **path,
zend_string **domain, zend_bool *secure, zend_bool *httponly, zend_string **samesite)
{
zend_string *key;
zval *value;

ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(options), key, value) {
if (key) {
if (zend_string_equals_literal_ci(key, "expires")) {
*expires = zval_get_long(value);
found++;
} else if (zend_string_equals_literal_ci(key, "path")) {
*path = zval_get_string(value);
found++;
} else if (zend_string_equals_literal_ci(key, "domain")) {
*domain = zval_get_string(value);
found++;
} else if (zend_string_equals_literal_ci(key, "secure")) {
*secure = zval_is_true(value);
found++;
} else if (zend_string_equals_literal_ci(key, "httponly")) {
*httponly = zval_is_true(value);
found++;
} else if (zend_string_equals_literal_ci(key, "samesite")) {
*samesite = zval_get_string(value);
found++;
} else {
php_error_docref(NULL, E_WARNING, "Unrecognized key '%s' found in the options array", ZSTR_VAL(key));
}
if (!key) {
zend_value_error("%s(): option array cannot have numeric keys", get_active_function_name());
return FAILURE;
}
if (zend_string_equals_literal_ci(key, "expires")) {
*expires = zval_get_long(value);
} else if (zend_string_equals_literal_ci(key, "path")) {
*path = zval_get_string(value);
} else if (zend_string_equals_literal_ci(key, "domain")) {
*domain = zval_get_string(value);
} else if (zend_string_equals_literal_ci(key, "secure")) {
*secure = zval_is_true(value);
} else if (zend_string_equals_literal_ci(key, "httponly")) {
*httponly = zval_is_true(value);
} else if (zend_string_equals_literal_ci(key, "samesite")) {
*samesite = zval_get_string(value);
} else {
php_error_docref(NULL, E_WARNING, "Numeric key found in the options array");
zend_value_error("%s(): option \"%s\" is invalid", get_active_function_name(), ZSTR_VAL(key));
return FAILURE;
}
} ZEND_HASH_FOREACH_END();

/* Array is not empty but no valid keys were found */
if (found == 0 && zend_hash_num_elements(Z_ARRVAL_P(options)) > 0) {
php_error_docref(NULL, E_WARNING, "No valid options were found in the given array");
}
return SUCCESS;
}

/* {{{ setcookie(string name [, string value [, array options]])
Send a cookie */
PHP_FUNCTION(setcookie)
static void php_setcookie_common(INTERNAL_FUNCTION_PARAMETERS, bool is_raw)
{
/* to handle overloaded function array|int */
zval *expires_or_options = NULL;
zend_string *name, *value = NULL, *path = NULL, *domain = NULL, *samesite = NULL;
zend_long expires = 0;
Expand All @@ -248,24 +245,27 @@ PHP_FUNCTION(setcookie)
if (expires_or_options) {
if (Z_TYPE_P(expires_or_options) == IS_ARRAY) {
if (UNEXPECTED(ZEND_NUM_ARGS() > 3)) {
php_error_docref(NULL, E_WARNING, "Cannot pass arguments after the options array");
RETURN_FALSE;
zend_argument_count_error("%s(): Expects exactly 3 arguments when argument #3 "
"($expires_or_options) is an array", get_active_function_name());
RETURN_THROWS();
}
if (FAILURE == php_head_parse_cookie_options_array(expires_or_options, &expires, &path,
&domain, &secure, &httponly, &samesite)) {
goto cleanup;
}
php_head_parse_cookie_options_array(expires_or_options, &expires, &path, &domain, &secure, &httponly, &samesite);
} else {
expires = zval_get_long(expires_or_options);
}
}

if (!EG(exception)) {
if (php_setcookie(name, value, expires, path, domain, secure, httponly, samesite, 1) == SUCCESS) {
RETVAL_TRUE;
} else {
RETVAL_FALSE;
}
if (php_setcookie(name, value, expires, path, domain, secure, httponly, samesite, !is_raw) == SUCCESS) {
RETVAL_TRUE;
} else {
RETVAL_FALSE;
}

if (expires_or_options && Z_TYPE_P(expires_or_options) == IS_ARRAY) {
cleanup:
if (path) {
zend_string_release(path);
}
Expand All @@ -277,59 +277,20 @@ PHP_FUNCTION(setcookie)
}
}
}

/* {{{ setcookie(string name [, string value [, array options]])
Send a cookie */
PHP_FUNCTION(setcookie)
{
php_setcookie_common(INTERNAL_FUNCTION_PARAM_PASSTHRU, false);
}
/* }}} */

/* {{{ setrawcookie(string name [, string value [, array options]])
Send a cookie with no url encoding of the value */
PHP_FUNCTION(setrawcookie)
{
zval *expires_or_options = NULL;
zend_string *name, *value = NULL, *path = NULL, *domain = NULL, *samesite = NULL;
zend_long expires = 0;
zend_bool secure = 0, httponly = 0;

ZEND_PARSE_PARAMETERS_START(1, 7)
Z_PARAM_STR(name)
Z_PARAM_OPTIONAL
Z_PARAM_STR(value)
Z_PARAM_ZVAL(expires_or_options)
Z_PARAM_STR(path)
Z_PARAM_STR(domain)
Z_PARAM_BOOL(secure)
Z_PARAM_BOOL(httponly)
ZEND_PARSE_PARAMETERS_END();

if (expires_or_options) {
if (Z_TYPE_P(expires_or_options) == IS_ARRAY) {
if (UNEXPECTED(ZEND_NUM_ARGS() > 3)) {
php_error_docref(NULL, E_WARNING, "Cannot pass arguments after the options array");
RETURN_FALSE;
}
php_head_parse_cookie_options_array(expires_or_options, &expires, &path, &domain, &secure, &httponly, &samesite);
} else {
expires = zval_get_long(expires_or_options);
}
}

if (!EG(exception)) {
if (php_setcookie(name, value, expires, path, domain, secure, httponly, samesite, 0) == SUCCESS) {
RETVAL_TRUE;
} else {
RETVAL_FALSE;
}
}

if (expires_or_options && Z_TYPE_P(expires_or_options) == IS_ARRAY) {
if (path) {
zend_string_release(path);
}
if (domain) {
zend_string_release(domain);
}
if (samesite) {
zend_string_release(samesite);
}
}
php_setcookie_common(INTERNAL_FUNCTION_PARAM_PASSTHRU, true);
}
/* }}} */

Expand Down
4 changes: 3 additions & 1 deletion ext/standard/head.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
extern PHP_RINIT_FUNCTION(head);

PHPAPI int php_header(void);
PHPAPI int php_setcookie(zend_string *name, zend_string *value, time_t expires, zend_string *path, zend_string *domain, int secure, int httponly, zend_string *samesite, int url_encode);
PHPAPI zend_result php_setcookie(zend_string *name, zend_string *value, time_t expires,
zend_string *path, zend_string *domain, bool secure, bool httponly,
zend_string *samesite, bool url_encode);

#endif
10 changes: 7 additions & 3 deletions ext/standard/tests/network/bug69523.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@
setcookie() allows empty cookie name
--FILE--
<?php
setcookie('', 'foo');
try {
setcookie('', 'foo');
} catch (\ValueError $e) {
echo $e->getMessage() . \PHP_EOL;
}
?>
--EXPECTF--
Warning: Cookie names must not be empty in %s on line %d
--EXPECT--
setcookie(): Argument #1 ($name) cannot be empty
24 changes: 14 additions & 10 deletions ext/standard/tests/network/bug69948.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,21 @@
Bug #69948 (path/domain are not sanitized for special characters in setcookie)
--FILE--
<?php
var_dump(
setcookie('foo', 'bar', 0, 'asdf;asdf'),
setcookie('foo', 'bar', 0, '/', 'foobar; secure')
);
try {
var_dump(setcookie('foo', 'bar', 0, 'asdf;asdf'));
} catch (\ValueError $e) {
echo $e->getMessage() . \PHP_EOL;
}
try {
var_dump(setcookie('foo', 'bar', 0, '/', 'foobar; secure'));
} catch (\ValueError $e) {
echo $e->getMessage() . \PHP_EOL;
}

?>
===DONE===
--EXPECTHEADERS--
--EXPECTF--
Warning: Cookie paths cannot contain any of the following ',; \t\r\n\013\014' in %s on line %d

Warning: Cookie domains cannot contain any of the following ',; \t\r\n\013\014' in %s on line %d
bool(false)
bool(false)
--EXPECT--
setcookie(): "path" option cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
setcookie(): "domain" option cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
===DONE===
69 changes: 69 additions & 0 deletions ext/standard/tests/network/setcookie_array_option_error.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
--TEST--
setcookie() array variant error tests
--INI--
date.timezone=UTC
--FILE--
<?php

ob_start();

// Unrecognized key and no valid keys
try {
setcookie('name', 'value', ['unknown_key' => 'only']);
} catch (\ValueError $e) {
echo $e->getMessage() . "\n";
}
// Numeric key and no valid keys
try {
setcookie('name2', 'value2', [0 => 'numeric_key']);
} catch (\ValueError $e) {
echo $e->getMessage() . "\n";
}
// Unrecognized key
try {
setcookie('name3', 'value3', ['path' => '/path/', 'foo' => 'bar']);
} catch (\ValueError $e) {
echo $e->getMessage() . "\n";
}
// Invalid expiration date
// To go above year 9999: 60 * 60 * 24 * 365 * 9999
try {
setcookie('name', 'value', ['expires' => 315328464000]);
} catch (\ValueError $e) {
echo $e->getMessage() . "\n";
}
// Invalid path key content
try {
setcookie('name', 'value', ['path' => '/;/']);
} catch (\ValueError $e) {
echo $e->getMessage() . "\n";
}
// Invalid domain key content
try {
setcookie('name', 'value', ['path' => '/path/', 'domain' => 'ba;r']);
} catch (\ValueError $e) {
echo $e->getMessage() . "\n";
}

// Arguments after options array (will not be set)
try {
setcookie('name4', 'value4', [], "path", "domain.tld", true, true);
} catch (\ArgumentCountError $e) {
echo $e->getMessage() . "\n";
}

var_dump(headers_list());
--EXPECTHEADERS--

--EXPECTF--
setcookie(): option "unknown_key" is invalid
setcookie(): option array cannot have numeric keys
setcookie(): option "foo" is invalid
setcookie(): "expires" option cannot have a year greater than 9999
setcookie(): "path" option cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
setcookie(): "domain" option cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
setcookie(): Expects exactly 3 arguments when argument #3 ($expires_or_options) is an array
array(1) {
[0]=>
string(%s) "X-Powered-By: PHP/%s"
}
Loading