Skip to content

Commit ce7ecaa

Browse files
committed
Fix zend_string leak on case-variant duplicate setcookie() options
php_head_parse_cookie_options_array() matches option keys case insensitively, but array keys are case sensitive, so a duplicate differing only in case (e.g. "path" and "Path") overwrote the previously fetched path/domain/samesite string without releasing it. Release any value already stored before fetching the next one.
1 parent 9e9b309 commit ce7ecaa

2 files changed

Lines changed: 24 additions & 0 deletions

File tree

ext/standard/head.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,14 +205,23 @@ static zend_result php_head_parse_cookie_options_array(HashTable *options, zend_
205205
if (zend_string_equals_literal_ci(key, "expires")) {
206206
*expires = zval_get_long(value);
207207
} else if (zend_string_equals_literal_ci(key, "path")) {
208+
if (*path) {
209+
zend_string_release(*path);
210+
}
208211
*path = zval_get_string(value);
209212
} else if (zend_string_equals_literal_ci(key, "domain")) {
213+
if (*domain) {
214+
zend_string_release(*domain);
215+
}
210216
*domain = zval_get_string(value);
211217
} else if (zend_string_equals_literal_ci(key, "secure")) {
212218
*secure = zval_is_true(value);
213219
} else if (zend_string_equals_literal_ci(key, "httponly")) {
214220
*httponly = zval_is_true(value);
215221
} else if (zend_string_equals_literal_ci(key, "samesite")) {
222+
if (*samesite) {
223+
zend_string_release(*samesite);
224+
}
216225
*samesite = zval_get_string(value);
217226
} else {
218227
zend_value_error("%s(): option \"%s\" is invalid", get_active_function_name(), ZSTR_VAL(key));
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
--TEST--
2+
setcookie() does not leak when an option array has case-variant duplicate keys
3+
--FILE--
4+
<?php
5+
$base = memory_get_usage();
6+
for ($i = 0; $i < 5000; $i++) {
7+
@setcookie('name', 'value', ['path' => '/aaaaaaaaaaaaaaaa' . $i, 'Path' => '/bbbbbbbbbbbbbbbb' . $i]);
8+
header_remove();
9+
}
10+
// Each duplicate-key call leaked the first path string before the fix,
11+
// growing usage by tens of bytes per iteration (hundreds of KB here).
12+
var_dump(memory_get_usage() - $base < 50000);
13+
?>
14+
--EXPECT--
15+
bool(true)

0 commit comments

Comments
 (0)