Skip to content

Commit c1691bf

Browse files
GirgiasGirgias
committed
Warning to error promotion for set(raw)cookie()
1 parent 9e8a8e6 commit c1691bf

File tree

6 files changed

+213
-116
lines changed

6 files changed

+213
-116
lines changed

‎ext/standard/head.c

+51-77
Original file line numberDiff line numberDiff line change
@@ -83,30 +83,6 @@ PHPAPI int php_setcookie(zend_string *name, zend_string *value, time_t expires,
8383
int result;
8484
smart_str buf = {0};
8585

86-
if (!ZSTR_LEN(name)) {
87-
zend_error( E_WARNING, "Cookie names must not be empty" );
88-
return FAILURE;
89-
} else if (strpbrk(ZSTR_VAL(name), "=,; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
90-
zend_error(E_WARNING, "Cookie names cannot contain any of the following '=,; \\t\\r\\n\\013\\014'" );
91-
return FAILURE;
92-
}
93-
94-
if (!url_encode && value &&
95-
strpbrk(ZSTR_VAL(value), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
96-
zend_error(E_WARNING, "Cookie values cannot contain any of the following ',; \\t\\r\\n\\013\\014'" );
97-
return FAILURE;
98-
}
99-
100-
if (path && strpbrk(ZSTR_VAL(path), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
101-
zend_error(E_WARNING, "Cookie paths cannot contain any of the following ',; \\t\\r\\n\\013\\014'" );
102-
return FAILURE;
103-
}
104-
105-
if (domain && strpbrk(ZSTR_VAL(domain), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
106-
zend_error(E_WARNING, "Cookie domains cannot contain any of the following ',; \\t\\r\\n\\013\\014'" );
107-
return FAILURE;
108-
}
109-
11086
if (value == NULL || ZSTR_LEN(value) == 0) {
11187
/*
11288
* MSIE doesn't delete a cookie when you set it to a null value
@@ -225,10 +201,10 @@ static void php_head_parse_cookie_options_array(zval *options, zend_long *expire
225201
}
226202
}
227203

228-
/* {{{ setcookie(string name [, string value [, array options]])
229-
Send a cookie */
230-
PHP_FUNCTION(setcookie)
204+
#define ILLEGAL_COOKIE_CHARACTER "\",\", \";\", \" \", \"\\t\", \"\\r\", \"\\n\", \"\\013\", and \"\\014\""
205+
static void php_setcookie_common(INTERNAL_FUNCTION_PARAMETERS, bool is_raw)
231206
{
207+
/* to handle overloaded function array|int */
232208
zval *expires_or_options = NULL;
233209
zend_string *name, *value = NULL, *path = NULL, *domain = NULL, *samesite = NULL;
234210
zend_long expires = 0;
@@ -248,24 +224,61 @@ PHP_FUNCTION(setcookie)
248224
if (expires_or_options) {
249225
if (Z_TYPE_P(expires_or_options) == IS_ARRAY) {
250226
if (UNEXPECTED(ZEND_NUM_ARGS() > 3)) {
251-
php_error_docref(NULL, E_WARNING, "Cannot pass arguments after the options array");
252-
RETURN_FALSE;
227+
zend_argument_count_error("%s(): Expects exactly 3 arguments when argument #3 "
228+
"($expires_or_options) is an array", get_active_function_name());
229+
RETURN_THROWS();
253230
}
254231
php_head_parse_cookie_options_array(expires_or_options, &expires, &path, &domain, &secure, &httponly, &samesite);
232+
if (path && strpbrk(ZSTR_VAL(path), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
233+
zend_value_error("%s(): Argument #3 ($expires_or_options[\"path\"]) cannot contain "
234+
ILLEGAL_COOKIE_CHARACTER, get_active_function_name());
235+
goto cleanup;
236+
RETURN_THROWS();
237+
}
238+
if (domain && strpbrk(ZSTR_VAL(domain), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
239+
zend_value_error("%s(): Argument #3 ($expires_or_options[\"domain\"]) cannot contain "
240+
ILLEGAL_COOKIE_CHARACTER, get_active_function_name());
241+
goto cleanup;
242+
RETURN_THROWS();
243+
}
244+
/* Should check value of SameSite? */
255245
} else {
256246
expires = zval_get_long(expires_or_options);
247+
if (path && strpbrk(ZSTR_VAL(path), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
248+
zend_argument_value_error(4, "cannot contain " ILLEGAL_COOKIE_CHARACTER);
249+
RETURN_THROWS();
250+
}
251+
if (domain && strpbrk(ZSTR_VAL(domain), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
252+
zend_argument_value_error(5, "cannot contain " ILLEGAL_COOKIE_CHARACTER);
253+
RETURN_THROWS();
254+
}
257255
}
258256
}
259257

258+
if (!ZSTR_LEN(name)) {
259+
zend_argument_value_error(1, "cannot be empty");
260+
RETURN_THROWS();
261+
}
262+
if (strpbrk(ZSTR_VAL(name), "=,; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
263+
zend_argument_value_error(1, "cannot contain \"=\", " ILLEGAL_COOKIE_CHARACTER);
264+
RETURN_THROWS();
265+
}
266+
if (is_raw && value &&
267+
strpbrk(ZSTR_VAL(value), ",; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
268+
zend_argument_value_error(2, "cannot contain " ILLEGAL_COOKIE_CHARACTER);
269+
RETURN_THROWS();
270+
}
271+
260272
if (!EG(exception)) {
261-
if (php_setcookie(name, value, expires, path, domain, secure, httponly, samesite, 1) == SUCCESS) {
273+
if (php_setcookie(name, value, expires, path, domain, secure, httponly, samesite, !is_raw) == SUCCESS) {
262274
RETVAL_TRUE;
263275
} else {
264276
RETVAL_FALSE;
265277
}
266278
}
267279

268280
if (expires_or_options && Z_TYPE_P(expires_or_options) == IS_ARRAY) {
281+
cleanup:
269282
if (path) {
270283
zend_string_release(path);
271284
}
@@ -277,59 +290,20 @@ PHP_FUNCTION(setcookie)
277290
}
278291
}
279292
}
293+
294+
/* {{{ setcookie(string name [, string value [, array options]])
295+
Send a cookie */
296+
PHP_FUNCTION(setcookie)
297+
{
298+
php_setcookie_common(INTERNAL_FUNCTION_PARAM_PASSTHRU, false);
299+
}
280300
/* }}} */
281301

282302
/* {{{ setrawcookie(string name [, string value [, array options]])
283303
Send a cookie with no url encoding of the value */
284304
PHP_FUNCTION(setrawcookie)
285305
{
286-
zval *expires_or_options = NULL;
287-
zend_string *name, *value = NULL, *path = NULL, *domain = NULL, *samesite = NULL;
288-
zend_long expires = 0;
289-
zend_bool secure = 0, httponly = 0;
290-
291-
ZEND_PARSE_PARAMETERS_START(1, 7)
292-
Z_PARAM_STR(name)
293-
Z_PARAM_OPTIONAL
294-
Z_PARAM_STR(value)
295-
Z_PARAM_ZVAL(expires_or_options)
296-
Z_PARAM_STR(path)
297-
Z_PARAM_STR(domain)
298-
Z_PARAM_BOOL(secure)
299-
Z_PARAM_BOOL(httponly)
300-
ZEND_PARSE_PARAMETERS_END();
301-
302-
if (expires_or_options) {
303-
if (Z_TYPE_P(expires_or_options) == IS_ARRAY) {
304-
if (UNEXPECTED(ZEND_NUM_ARGS() > 3)) {
305-
php_error_docref(NULL, E_WARNING, "Cannot pass arguments after the options array");
306-
RETURN_FALSE;
307-
}
308-
php_head_parse_cookie_options_array(expires_or_options, &expires, &path, &domain, &secure, &httponly, &samesite);
309-
} else {
310-
expires = zval_get_long(expires_or_options);
311-
}
312-
}
313-
314-
if (!EG(exception)) {
315-
if (php_setcookie(name, value, expires, path, domain, secure, httponly, samesite, 0) == SUCCESS) {
316-
RETVAL_TRUE;
317-
} else {
318-
RETVAL_FALSE;
319-
}
320-
}
321-
322-
if (expires_or_options && Z_TYPE_P(expires_or_options) == IS_ARRAY) {
323-
if (path) {
324-
zend_string_release(path);
325-
}
326-
if (domain) {
327-
zend_string_release(domain);
328-
}
329-
if (samesite) {
330-
zend_string_release(samesite);
331-
}
332-
}
306+
php_setcookie_common(INTERNAL_FUNCTION_PARAM_PASSTHRU, true);
333307
}
334308
/* }}} */
335309

‎ext/standard/tests/network/bug69523.phpt

+7-3
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@
22
setcookie() allows empty cookie name
33
--FILE--
44
<?php
5-
setcookie('', 'foo');
5+
try {
6+
setcookie('', 'foo');
7+
} catch (\ValueError $e) {
8+
echo $e->getMessage() . \PHP_EOL;
9+
}
610
?>
7-
--EXPECTF--
8-
Warning: Cookie names must not be empty in %s on line %d
11+
--EXPECT--
12+
setcookie(): Argument #1 ($name) cannot be empty

‎ext/standard/tests/network/bug69948.phpt

+14-10
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,21 @@
22
Bug #69948 (path/domain are not sanitized for special characters in setcookie)
33
--FILE--
44
<?php
5-
var_dump(
6-
setcookie('foo', 'bar', 0, 'asdf;asdf'),
7-
setcookie('foo', 'bar', 0, '/', 'foobar; secure')
8-
);
5+
try {
6+
var_dump(setcookie('foo', 'bar', 0, 'asdf;asdf'));
7+
} catch (\ValueError $e) {
8+
echo $e->getMessage() . \PHP_EOL;
9+
}
10+
try {
11+
var_dump(setcookie('foo', 'bar', 0, '/', 'foobar; secure'));
12+
} catch (\ValueError $e) {
13+
echo $e->getMessage() . \PHP_EOL;
14+
}
15+
916
?>
1017
===DONE===
1118
--EXPECTHEADERS--
12-
--EXPECTF--
13-
Warning: Cookie paths cannot contain any of the following ',; \t\r\n\013\014' in %s on line %d
14-
15-
Warning: Cookie domains cannot contain any of the following ',; \t\r\n\013\014' in %s on line %d
16-
bool(false)
17-
bool(false)
19+
--EXPECT--
20+
setcookie(): Argument #4 ($path) cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
21+
setcookie(): Argument #5 ($domain) cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
1822
===DONE===

‎ext/standard/tests/network/setcookie_array_option_error.phpt

+61
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
--TEST--
2+
setcookie() array variant error tests
3+
--INI--
4+
date.timezone=UTC
5+
--FILE--
6+
<?php
7+
8+
ob_start();
9+
10+
// Unrecognized key and no valid keys
11+
setcookie('name', 'value', ['unknown_key' => 'only']);
12+
// Numeric key and no valid keys
13+
setcookie('name2', 'value2', [0 => 'numeric_key']);
14+
// Unrecognized key
15+
setcookie('name3', 'value3', ['path' => '/path/', 'foo' => 'bar']);
16+
// Invalid path key content
17+
try {
18+
setcookie('name', 'value', ['path' => '/;/']);
19+
} catch (\ValueError $e) {
20+
echo $e->getMessage() . "\n";
21+
}
22+
// Invalid domain key content
23+
try {
24+
setcookie('name', 'value', ['path' => '/path/', 'domain' => 'ba;r']);
25+
} catch (\ValueError $e) {
26+
echo $e->getMessage() . "\n";
27+
}
28+
29+
// Arguments after options array (will not be set)
30+
try {
31+
setcookie('name4', 'value4', [], "path", "domain.tld", true, true);
32+
} catch (\ArgumentCountError $e) {
33+
echo $e->getMessage() . "\n";
34+
}
35+
36+
var_dump(headers_list());
37+
--EXPECTHEADERS--
38+
39+
--EXPECTF--
40+
Warning: setcookie(): Unrecognized key 'unknown_key' found in the options array in %s
41+
42+
Warning: setcookie(): No valid options were found in the given array in %s
43+
44+
Warning: setcookie(): Numeric key found in the options array in %s
45+
46+
Warning: setcookie(): No valid options were found in the given array in %s
47+
48+
Warning: setcookie(): Unrecognized key 'foo' found in the options array in %s
49+
setcookie(): Argument #3 ($expires_or_options["path"]) cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
50+
setcookie(): Argument #3 ($expires_or_options["domain"]) cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
51+
setcookie(): Expects exactly 3 arguments when argument #3 ($expires_or_options) is an array
52+
array(4) {
53+
[0]=>
54+
string(%d) "X-Powered-By: PHP/%s"
55+
[1]=>
56+
string(22) "Set-Cookie: name=value"
57+
[2]=>
58+
string(24) "Set-Cookie: name2=value2"
59+
[3]=>
60+
string(37) "Set-Cookie: name3=value3; path=/path/"
61+
}

‎ext/standard/tests/network/setcookie_error.phpt

+32-26
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,49 @@
11
--TEST--
2-
setcookie() array variant error tests
2+
setcookie() error tests
33
--INI--
44
date.timezone=UTC
55
--FILE--
66
<?php
77

88
ob_start();
99

10-
// Unrecognized key and no valid keys
11-
setcookie('name', 'value', ['unknown_key' => 'only']);
12-
// Numeric key and no valid keys
13-
setcookie('name2', 'value2', [0 => 'numeric_key']);
14-
// Unrecognized key
15-
setcookie('name3', 'value3', ['path' => '/path/', 'foo' => 'bar']);
16-
// Arguments after options array (will not be set)
17-
setcookie('name4', 'value4', [], "path", "domain.tld", true, true);
10+
try {
11+
setcookie('');
12+
} catch (\ValueError $e) {
13+
echo $e->getMessage() . "\n";
14+
}
15+
try {
16+
setcookie('invalid=');
17+
} catch (\ValueError $e) {
18+
echo $e->getMessage() . "\n";
19+
}
20+
try {
21+
setcookie('name', 'invalid;');
22+
} catch (\ValueError $e) {
23+
echo $e->getMessage() . "\n";
24+
}
25+
try {
26+
setcookie('name', 'value', 100, 'invalid;');
27+
} catch (\ValueError $e) {
28+
echo $e->getMessage() . "\n";
29+
}
30+
try {
31+
setcookie('name', 'value', 100, 'path', 'invalid;');
32+
} catch (\ValueError $e) {
33+
echo $e->getMessage() . "\n";
34+
}
1835

1936
var_dump(headers_list());
2037
--EXPECTHEADERS--
2138

2239
--EXPECTF--
23-
Warning: setcookie(): Unrecognized key 'unknown_key' found in the options array in %s
24-
25-
Warning: setcookie(): No valid options were found in the given array in %s
26-
27-
Warning: setcookie(): Numeric key found in the options array in %s
28-
29-
Warning: setcookie(): No valid options were found in the given array in %s
30-
31-
Warning: setcookie(): Unrecognized key 'foo' found in the options array in %s
32-
33-
Warning: setcookie(): Cannot pass arguments after the options array in %s
34-
array(4) {
40+
setcookie(): Argument #1 ($name) cannot be empty
41+
setcookie(): Argument #1 ($name) cannot contain "=", ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
42+
setcookie(): Argument #4 ($path) cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
43+
setcookie(): Argument #5 ($domain) cannot contain ",", ";", " ", "\t", "\r", "\n", "\013", and "\014"
44+
array(2) {
3545
[0]=>
3646
string(%d) "X-Powered-By: PHP/%s"
3747
[1]=>
38-
string(22) "Set-Cookie: name=value"
39-
[2]=>
40-
string(24) "Set-Cookie: name2=value2"
41-
[3]=>
42-
string(37) "Set-Cookie: name3=value3; path=/path/"
48+
string(27) "Set-Cookie: name=invalid%3B"
4349
}

0 commit comments

Comments
 (0)