Skip to content

Commit 2124ae0

Browse files
GirgiasGirgias
committed
Convert session_name from char to zend_string
Also fixed a bug where nul bytes where not properly checked
1 parent ed19c52 commit 2124ae0

File tree

6 files changed

+57
-41
lines changed

6 files changed

+57
-41
lines changed

‎Zend/Optimizer/zend_func_infos.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,6 @@ static const func_info_t func_infos[] = {
419419
#if defined(HAVE_HISTORY_LIST)
420420
F1("readline_list_history", MAY_BE_ARRAY|MAY_BE_ARRAY_KEY_LONG|MAY_BE_ARRAY_OF_STRING),
421421
#endif
422-
F1("session_name", MAY_BE_STRING|MAY_BE_FALSE),
423422
F1("session_module_name", MAY_BE_STRING|MAY_BE_FALSE),
424423
F1("session_save_path", MAY_BE_STRING|MAY_BE_FALSE),
425424
F1("session_create_id", MAY_BE_STRING|MAY_BE_FALSE),

‎ext/session/php_session.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ typedef struct _php_session_rfc1867_progress {
141141

142142
typedef struct _php_ps_globals {
143143
char *save_path;
144-
char *session_name;
144+
zend_string *session_name;
145145
zend_string *id;
146146
zend_string *extern_referer_chk;
147147
zend_string *cache_limiter;

‎ext/session/session.c

Lines changed: 51 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ static zend_result php_session_initialize(void) /* {{{ */
390390
}
391391

392392
/* Open session handler first */
393-
if (PS(mod)->s_open(&PS(mod_data), PS(save_path), PS(session_name)) == FAILURE
393+
if (PS(mod)->s_open(&PS(mod_data), PS(save_path), ZSTR_VAL(PS(session_name))) == FAILURE
394394
/* || PS(mod_data) == NULL */ /* FIXME: open must set valid PS(mod_data) with success */
395395
) {
396396
php_session_abort();
@@ -664,24 +664,41 @@ static PHP_INI_MH(OnUpdateName) /* {{{ */
664664
SESSION_CHECK_ACTIVE_STATE;
665665
SESSION_CHECK_OUTPUT_STATE;
666666

667-
/* Numeric session.name won't work at all */
668-
if ((!ZSTR_LEN(new_value) || is_numeric_string(ZSTR_VAL(new_value), ZSTR_LEN(new_value), NULL, NULL, 0))) {
669-
int err_type;
667+
int err_type;
670668

671-
if (stage == ZEND_INI_STAGE_RUNTIME || stage == ZEND_INI_STAGE_ACTIVATE || stage == ZEND_INI_STAGE_STARTUP) {
672-
err_type = E_WARNING;
673-
} else {
674-
err_type = E_ERROR;
675-
}
669+
if (stage == ZEND_INI_STAGE_RUNTIME || stage == ZEND_INI_STAGE_ACTIVATE || stage == ZEND_INI_STAGE_STARTUP) {
670+
err_type = E_WARNING;
671+
} else {
672+
err_type = E_ERROR;
673+
}
676674

675+
if (ZSTR_LEN(new_value) == 0) {
676+
/* Do not output error when restoring ini options. */
677+
if (stage != ZEND_INI_STAGE_DEACTIVATE) {
678+
php_error_docref(NULL, err_type, "session.name \"%s\" cannot be empty", ZSTR_VAL(new_value));
679+
}
680+
return FAILURE;
681+
}
682+
/* Nul bytes are not allowed */
683+
if (ZSTR_LEN(new_value) != strlen(ZSTR_VAL(new_value))) {
684+
/* Do not output error when restoring ini options. */
685+
if (stage != ZEND_INI_STAGE_DEACTIVATE) {
686+
php_error_docref(NULL, err_type, "session.name \"%s\" cannot contain nul bytes", ZSTR_VAL(new_value));
687+
}
688+
return FAILURE;
689+
}
690+
/* Numeric session.name won't work at all */
691+
if (is_numeric_str_function(new_value, NULL, NULL)) {
677692
/* Do not output error when restoring ini options. */
678693
if (stage != ZEND_INI_STAGE_DEACTIVATE) {
679694
php_error_docref(NULL, err_type, "session.name \"%s\" cannot be numeric or empty", ZSTR_VAL(new_value));
680695
}
681696
return FAILURE;
682697
}
683698

684-
return OnUpdateStringUnempty(entry, new_value, mh_arg1, mh_arg2, mh_arg3, stage);
699+
zend_string **p = (zend_string **) ZEND_INI_GET_ADDR();
700+
*p = new_value;
701+
return SUCCESS;
685702
}
686703
/* }}} */
687704

@@ -1278,9 +1295,10 @@ static void php_session_remove_cookie(void) {
12781295
size_t session_cookie_len;
12791296
size_t len = sizeof("Set-Cookie")-1;
12801297

1281-
ZEND_ASSERT(strpbrk(PS(session_name), "=,; \t\r\n\013\014") == NULL);
1282-
spprintf(&session_cookie, 0, "Set-Cookie: %s=", PS(session_name));
1298+
ZEND_ASSERT(strpbrk(ZSTR_VAL(PS(session_name)), "=,; \t\r\n\013\014") == NULL);
1299+
spprintf(&session_cookie, 0, "Set-Cookie: %s=", ZSTR_VAL(PS(session_name)));
12831300

1301+
// TODO Manually compute from known information?
12841302
session_cookie_len = strlen(session_cookie);
12851303
current = l->head;
12861304
while (current) {
@@ -1325,8 +1343,9 @@ static zend_result php_session_send_cookie(void) /* {{{ */
13251343
return FAILURE;
13261344
}
13271345

1346+
// TODO need to Check for nul byte?
13281347
/* Prevent broken Set-Cookie header, because the session_name might be user supplied */
1329-
if (strpbrk(PS(session_name), "=,; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
1348+
if (strpbrk(ZSTR_VAL(PS(session_name)), "=,; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
13301349
php_error_docref(NULL, E_WARNING, "session.name cannot contain any of the following '=,; \\t\\r\\n\\013\\014'");
13311350
return FAILURE;
13321351
}
@@ -1335,7 +1354,7 @@ static zend_result php_session_send_cookie(void) /* {{{ */
13351354
e_id = php_url_encode(ZSTR_VAL(PS(id)), ZSTR_LEN(PS(id)));
13361355

13371356
smart_str_appendl(&ncookie, "Set-Cookie: ", sizeof("Set-Cookie: ")-1);
1338-
smart_str_appendl(&ncookie, PS(session_name), strlen(PS(session_name)));
1357+
smart_str_append(&ncookie, PS(session_name));
13391358
smart_str_appendc(&ncookie, '=');
13401359
smart_str_appendl(&ncookie, ZSTR_VAL(e_id), ZSTR_LEN(e_id));
13411360

@@ -1461,7 +1480,7 @@ PHPAPI zend_result php_session_reset_id(void) /* {{{ */
14611480
if (PS(define_sid)) {
14621481
smart_str var = {0};
14631482

1464-
smart_str_appends(&var, PS(session_name));
1483+
smart_str_append(&var, PS(session_name));
14651484
smart_str_appendc(&var, '=');
14661485
smart_str_appends(&var, ZSTR_VAL(PS(id)));
14671486
smart_str_0(&var);
@@ -1489,18 +1508,15 @@ PHPAPI zend_result php_session_reset_id(void) /* {{{ */
14891508
(data = zend_hash_str_find(&EG(symbol_table), "_COOKIE", sizeof("_COOKIE") - 1))) {
14901509
ZVAL_DEREF(data);
14911510
if (Z_TYPE_P(data) == IS_ARRAY &&
1492-
(ppid = zend_hash_str_find(Z_ARRVAL_P(data), PS(session_name), strlen(PS(session_name))))) {
1511+
(ppid = zend_hash_find(Z_ARRVAL_P(data), PS(session_name)))) {
14931512
ZVAL_DEREF(ppid);
14941513
apply_trans_sid = 0;
14951514
}
14961515
}
14971516
}
14981517
if (apply_trans_sid) {
1499-
zend_string *sname;
1500-
sname = zend_string_init(PS(session_name), strlen(PS(session_name)), 0);
1501-
php_url_scanner_reset_session_var(sname, 1); /* This may fail when session name has changed */
1502-
zend_string_release_ex(sname, 0);
1503-
php_url_scanner_add_session_var(PS(session_name), strlen(PS(session_name)), ZSTR_VAL(PS(id)), ZSTR_LEN(PS(id)), 1);
1518+
php_url_scanner_reset_session_var(PS(session_name), 1); /* This may fail when session name has changed */
1519+
php_url_scanner_add_session_var(ZSTR_VAL(PS(session_name)), ZSTR_LEN(PS(session_name)), ZSTR_VAL(PS(id)), ZSTR_LEN(PS(id)), 1);
15041520
}
15051521
return SUCCESS;
15061522
}
@@ -1512,7 +1528,6 @@ PHPAPI zend_result php_session_start(void) /* {{{ */
15121528
zval *ppid;
15131529
zval *data;
15141530
char *p, *value;
1515-
size_t lensess;
15161531

15171532
switch (PS(session_status)) {
15181533
case php_session_active:
@@ -1547,8 +1562,6 @@ PHPAPI zend_result php_session_start(void) /* {{{ */
15471562
PS(send_cookie) = PS(use_cookies) || PS(use_only_cookies);
15481563
}
15491564

1550-
lensess = strlen(PS(session_name));
1551-
15521565
/*
15531566
* Cookies are preferred, because initially cookie and get
15541567
* variables will be available.
@@ -1560,7 +1573,7 @@ PHPAPI zend_result php_session_start(void) /* {{{ */
15601573
if (!PS(id)) {
15611574
if (PS(use_cookies) && (data = zend_hash_str_find(&EG(symbol_table), "_COOKIE", sizeof("_COOKIE") - 1))) {
15621575
ZVAL_DEREF(data);
1563-
if (Z_TYPE_P(data) == IS_ARRAY && (ppid = zend_hash_str_find(Z_ARRVAL_P(data), PS(session_name), lensess))) {
1576+
if (Z_TYPE_P(data) == IS_ARRAY && (ppid = zend_hash_find(Z_ARRVAL_P(data), PS(session_name)))) {
15641577
ppid2sid(ppid);
15651578
PS(send_cookie) = 0;
15661579
PS(define_sid) = 0;
@@ -1570,13 +1583,13 @@ PHPAPI zend_result php_session_start(void) /* {{{ */
15701583
if (!PS(use_only_cookies)) {
15711584
if (!PS(id) && (data = zend_hash_str_find(&EG(symbol_table), "_GET", sizeof("_GET") - 1))) {
15721585
ZVAL_DEREF(data);
1573-
if (Z_TYPE_P(data) == IS_ARRAY && (ppid = zend_hash_str_find(Z_ARRVAL_P(data), PS(session_name), lensess))) {
1586+
if (Z_TYPE_P(data) == IS_ARRAY && (ppid = zend_hash_find(Z_ARRVAL_P(data), PS(session_name)))) {
15741587
ppid2sid(ppid);
15751588
}
15761589
}
15771590
if (!PS(id) && (data = zend_hash_str_find(&EG(symbol_table), "_POST", sizeof("_POST") - 1))) {
15781591
ZVAL_DEREF(data);
1579-
if (Z_TYPE_P(data) == IS_ARRAY && (ppid = zend_hash_str_find(Z_ARRVAL_P(data), PS(session_name), lensess))) {
1592+
if (Z_TYPE_P(data) == IS_ARRAY && (ppid = zend_hash_find(Z_ARRVAL_P(data), PS(session_name)))) {
15801593
ppid2sid(ppid);
15811594
}
15821595
}
@@ -1586,11 +1599,11 @@ PHPAPI zend_result php_session_start(void) /* {{{ */
15861599
if (!PS(id) && zend_is_auto_global(ZSTR_KNOWN(ZEND_STR_AUTOGLOBAL_SERVER)) == SUCCESS &&
15871600
(data = zend_hash_str_find(Z_ARRVAL(PG(http_globals)[TRACK_VARS_SERVER]), "REQUEST_URI", sizeof("REQUEST_URI") - 1)) &&
15881601
Z_TYPE_P(data) == IS_STRING &&
1589-
(p = strstr(Z_STRVAL_P(data), PS(session_name))) &&
1590-
p[lensess] == '='
1602+
(p = strstr(Z_STRVAL_P(data), ZSTR_VAL(PS(session_name)))) &&
1603+
p[ZSTR_LEN(PS(session_name))] == '='
15911604
) {
15921605
char *q;
1593-
p += lensess + 1;
1606+
p += ZSTR_LEN(PS(session_name));
15941607
if ((q = strpbrk(p, "/?\\"))) {
15951608
PS(id) = zend_string_init(p, q - p, 0);
15961609
}
@@ -1671,7 +1684,7 @@ static zend_result php_session_reset(void) /* {{{ */
16711684
PHPAPI void session_adapt_url(const char *url, size_t url_len, char **new_url, size_t *new_len) /* {{{ */
16721685
{
16731686
if (APPLY_TRANS_SID && (PS(session_status) == php_session_active)) {
1674-
*new_url = php_url_scanner_adapt_single_url(url, url_len, PS(session_name), ZSTR_VAL(PS(id)), new_len, 1);
1687+
*new_url = php_url_scanner_adapt_single_url(url, url_len, ZSTR_VAL(PS(session_name)), ZSTR_VAL(PS(id)), new_len, 1);
16751688
}
16761689
}
16771690
/* }}} */
@@ -1892,7 +1905,8 @@ PHP_FUNCTION(session_name)
18921905
RETURN_FALSE;
18931906
}
18941907

1895-
RETVAL_STRING(PS(session_name));
1908+
// TODO Prevent duplication???
1909+
RETVAL_STR(zend_string_dup(PS(session_name), false));
18961910

18971911
if (name) {
18981912
ini_name = zend_string_init("session.name", sizeof("session.name") - 1, 0);
@@ -2267,7 +2281,7 @@ PHP_FUNCTION(session_regenerate_id)
22672281
zend_string_release_ex(PS(id), 0);
22682282
PS(id) = NULL;
22692283

2270-
if (PS(mod)->s_open(&PS(mod_data), PS(save_path), PS(session_name)) == FAILURE) {
2284+
if (PS(mod)->s_open(&PS(mod_data), PS(save_path), ZSTR_VAL(PS(session_name))) == FAILURE) {
22712285
PS(session_status) = php_session_none;
22722286
if (!EG(exception)) {
22732287
zend_throw_error(NULL, "Failed to open session: %s (path: %s)", PS(mod)->s_name, PS(save_path));
@@ -2935,7 +2949,7 @@ static bool early_find_sid_in(zval *dest, int where, php_session_rfc1867_progres
29352949
return 0;
29362950
}
29372951

2938-
if ((ppid = zend_hash_str_find(Z_ARRVAL(PG(http_globals)[where]), PS(session_name), progress->sname_len))
2952+
if ((ppid = zend_hash_find(Z_ARRVAL(PG(http_globals)[where]), PS(session_name)))
29392953
&& Z_TYPE_P(ppid) == IS_STRING) {
29402954
zval_ptr_dtor(dest);
29412955
ZVAL_COPY_DEREF(dest, ppid);
@@ -3043,7 +3057,8 @@ static zend_result php_session_rfc1867_callback(unsigned int event, void *event_
30433057
multipart_event_start *data = (multipart_event_start *) event_data;
30443058
progress = ecalloc(1, sizeof(php_session_rfc1867_progress));
30453059
progress->content_length = data->content_length;
3046-
progress->sname_len = strlen(PS(session_name));
3060+
// TODO Remove field?
3061+
progress->sname_len = ZSTR_LEN(PS(session_name));
30473062
PS(rfc1867_progress) = progress;
30483063
}
30493064
break;
@@ -3065,7 +3080,7 @@ static zend_result php_session_rfc1867_callback(unsigned int event, void *event_
30653080
if (data->name && data->value && value_len) {
30663081
size_t name_len = strlen(data->name);
30673082

3068-
if (name_len == progress->sname_len && memcmp(data->name, PS(session_name), name_len) == 0) {
3083+
if (name_len == progress->sname_len && memcmp(data->name, ZSTR_VAL(PS(session_name)), name_len) == 0) {
30693084
zval_ptr_dtor(&progress->sid);
30703085
ZVAL_STRINGL(&progress->sid, (*data->value), value_len);
30713086
} else if (name_len == strlen(PS(rfc1867_name)) && memcmp(data->name, PS(rfc1867_name), name_len + 1) == 0) {

‎ext/session/tests/bug66481.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,6 @@ var_dump(session_name("foo"));
1515
var_dump(session_name("bar"));
1616
?>
1717
--EXPECT--
18-
Warning: PHP Startup: session.name "" cannot be numeric or empty in Unknown on line 0
18+
Warning: PHP Startup: session.name "" cannot be empty in Unknown on line 0
1919
string(9) "PHPSESSID"
2020
string(3) "foo"

‎ext/session/tests/rfc1867_sid_post.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ var_dump($_SESSION["upload_progress_" . basename(__FILE__)]);
4747
session_destroy();
4848
?>
4949
--EXPECTF--
50-
string(%d) "rfc1867-sid-post"
50+
string(16) "rfc1867-sid-post"
5151
bool(true)
5252
array(2) {
5353
["file1"]=>

‎ext/session/tests/session_name_variation1.phpt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ ob_end_flush();
3838
?>
3939
--EXPECTF--
4040
*** Testing session_name() : variation ***
41+
42+
Warning: session_name(): session.name "" cannot contain nul bytes in %s on line %d
4143
string(9) "PHPSESSID"
4244
bool(true)
4345
string(9) "PHPSESSID"
@@ -51,7 +53,7 @@ string(1) " "
5153
bool(true)
5254
string(1) " "
5355

54-
Warning: session_name(): session.name "" cannot be numeric or empty in %s on line %d
56+
Warning: session_name(): session.name "" cannot be empty in %s on line %d
5557
string(1) " "
5658

5759
Warning: session_start(): session.name cannot contain any of the following '=,; \t\r\n\013\014' in %s on line %d

0 commit comments

Comments
 (0)