Skip to content

Commit 79b6f98

Browse files
Girgiasjorgsowa
Girgias
authored and
jorgsowa
committed
Convert session_name from char to zend_string
Also fixed a bug where nul bytes where not properly checked
1 parent 8e93cb1 commit 79b6f98

File tree

5 files changed

+71
-39
lines changed

5 files changed

+71
-39
lines changed

‎ext/session/php_session.h

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

141141
typedef struct _php_ps_globals {
142142
char *save_path;
143-
char *session_name;
143+
zend_string *session_name;
144144
zend_string *id;
145145
zend_string *extern_referer_chk;
146146
zend_string *cache_limiter;

‎ext/session/session.c

Lines changed: 65 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ static zend_result php_session_initialize(void) /* {{{ */
416416
}
417417

418418
/* Open session handler first */
419-
if (PS(mod)->s_open(&PS(mod_data), PS(save_path), PS(session_name)) == FAILURE
419+
if (PS(mod)->s_open(&PS(mod_data), PS(save_path), ZSTR_VAL(PS(session_name))) == FAILURE
420420
/* || PS(mod_data) == NULL */ /* FIXME: open must set valid PS(mod_data) with success */
421421
) {
422422
php_session_abort();
@@ -681,24 +681,41 @@ static PHP_INI_MH(OnUpdateName) /* {{{ */
681681
SESSION_CHECK_ACTIVE_STATE;
682682
SESSION_CHECK_OUTPUT_STATE;
683683

684-
/* Numeric session.name won't work at all */
685-
if ((!ZSTR_LEN(new_value) || is_numeric_string(ZSTR_VAL(new_value), ZSTR_LEN(new_value), NULL, NULL, 0))) {
686-
int err_type;
684+
int err_type;
687685

688-
if (stage == ZEND_INI_STAGE_RUNTIME || stage == ZEND_INI_STAGE_ACTIVATE || stage == ZEND_INI_STAGE_STARTUP) {
689-
err_type = E_WARNING;
690-
} else {
691-
err_type = E_ERROR;
692-
}
686+
if (stage == ZEND_INI_STAGE_RUNTIME || stage == ZEND_INI_STAGE_ACTIVATE || stage == ZEND_INI_STAGE_STARTUP) {
687+
err_type = E_WARNING;
688+
} else {
689+
err_type = E_ERROR;
690+
}
693691

692+
if (ZSTR_LEN(new_value) == 0) {
693+
/* Do not output error when restoring ini options. */
694+
if (stage != ZEND_INI_STAGE_DEACTIVATE) {
695+
php_error_docref(NULL, err_type, "session.name \"%s\" cannot be empty", ZSTR_VAL(new_value));
696+
}
697+
return FAILURE;
698+
}
699+
/* Nul bytes are not allowed */
700+
if (ZSTR_LEN(new_value) != strlen(ZSTR_VAL(new_value))) {
701+
/* Do not output error when restoring ini options. */
702+
if (stage != ZEND_INI_STAGE_DEACTIVATE) {
703+
php_error_docref(NULL, err_type, "session.name \"%s\" cannot contain nul bytes", ZSTR_VAL(new_value));
704+
}
705+
return FAILURE;
706+
}
707+
/* Numeric session.name won't work at all */
708+
if (is_numeric_str_function(new_value, NULL, NULL)) {
694709
/* Do not output error when restoring ini options. */
695710
if (stage != ZEND_INI_STAGE_DEACTIVATE) {
696711
php_error_docref(NULL, err_type, "session.name \"%s\" cannot be numeric or empty", ZSTR_VAL(new_value));
697712
}
698713
return FAILURE;
699714
}
700715

701-
return OnUpdateStringUnempty(entry, new_value, mh_arg1, mh_arg2, mh_arg3, stage);
716+
zend_string **p = (zend_string **) ZEND_INI_GET_ADDR();
717+
*p = new_value;
718+
return SUCCESS;
702719
}
703720
/* }}} */
704721

@@ -1372,9 +1389,10 @@ static void php_session_remove_cookie(void) {
13721389
size_t session_cookie_len;
13731390
size_t len = sizeof("Set-Cookie")-1;
13741391

1375-
ZEND_ASSERT(strpbrk(PS(session_name), SESSION_FORBIDDEN_CHARS) == NULL);
1376-
spprintf(&session_cookie, 0, "Set-Cookie: %s=", PS(session_name));
1392+
ZEND_ASSERT(strpbrk(ZSTR_VAL(PS(session_name)), SESSION_FORBIDDEN_CHARS) == NULL);
1393+
spprintf(&session_cookie, 0, "Set-Cookie: %s=", ZSTR_VAL(PS(session_name)));
13771394

1395+
// TODO Manually compute from known information?
13781396
session_cookie_len = strlen(session_cookie);
13791397
current = l->head;
13801398
while (current) {
@@ -1419,17 +1437,18 @@ static zend_result php_session_send_cookie(void) /* {{{ */
14191437
return FAILURE;
14201438
}
14211439

1440+
// TODO need to Check for nul byte?
14221441
/* Prevent broken Set-Cookie header, because the session_name might be user supplied */
1423-
if (strpbrk(PS(session_name), SESSION_FORBIDDEN_CHARS) != NULL) { /* man isspace for \013 and \014 */
1424-
php_error_docref(NULL, E_WARNING, "session.name cannot contain any of the following '=,;.[ \\t\\r\\n\\013\\014'");
1442+
if (strpbrk(ZSTR_VAL(PS(session_name)), SESSION_FORBIDDEN_CHARS) != NULL) { /* man isspace for \013 and \014 */
1443+
php_error_docref(NULL, E_WARNING, "session.name cannot contain any of the following '=,; \\t\\r\\n\\013\\014'");
14251444
return FAILURE;
14261445
}
14271446

14281447
/* URL encode id because it might be user supplied */
14291448
e_id = php_url_encode(ZSTR_VAL(PS(id)), ZSTR_LEN(PS(id)));
14301449

14311450
smart_str_appendl(&ncookie, "Set-Cookie: ", sizeof("Set-Cookie: ")-1);
1432-
smart_str_appendl(&ncookie, PS(session_name), strlen(PS(session_name)));
1451+
smart_str_append(&ncookie, PS(session_name));
14331452
smart_str_appendc(&ncookie, '=');
14341453
smart_str_appendl(&ncookie, ZSTR_VAL(e_id), ZSTR_LEN(e_id));
14351454

@@ -1555,7 +1574,7 @@ PHPAPI zend_result php_session_reset_id(void) /* {{{ */
15551574
if (PS(define_sid)) {
15561575
smart_str var = {0};
15571576

1558-
smart_str_appends(&var, PS(session_name));
1577+
smart_str_append(&var, PS(session_name));
15591578
smart_str_appendc(&var, '=');
15601579
smart_str_appends(&var, ZSTR_VAL(PS(id)));
15611580
smart_str_0(&var);
@@ -1583,18 +1602,15 @@ PHPAPI zend_result php_session_reset_id(void) /* {{{ */
15831602
(data = zend_hash_str_find(&EG(symbol_table), "_COOKIE", sizeof("_COOKIE") - 1))) {
15841603
ZVAL_DEREF(data);
15851604
if (Z_TYPE_P(data) == IS_ARRAY &&
1586-
(ppid = zend_hash_str_find(Z_ARRVAL_P(data), PS(session_name), strlen(PS(session_name))))) {
1605+
(ppid = zend_hash_find(Z_ARRVAL_P(data), PS(session_name)))) {
15871606
ZVAL_DEREF(ppid);
15881607
apply_trans_sid = 0;
15891608
}
15901609
}
15911610
}
15921611
if (apply_trans_sid) {
1593-
zend_string *sname;
1594-
sname = zend_string_init(PS(session_name), strlen(PS(session_name)), 0);
1595-
php_url_scanner_reset_session_var(sname, 1); /* This may fail when session name has changed */
1596-
zend_string_release_ex(sname, 0);
1597-
php_url_scanner_add_session_var(PS(session_name), strlen(PS(session_name)), ZSTR_VAL(PS(id)), ZSTR_LEN(PS(id)), 1);
1612+
php_url_scanner_reset_session_var(PS(session_name), 1); /* This may fail when session name has changed */
1613+
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);
15981614
}
15991615
return SUCCESS;
16001616
}
@@ -1605,8 +1621,7 @@ PHPAPI zend_result php_session_start(void) /* {{{ */
16051621
{
16061622
zval *ppid;
16071623
zval *data;
1608-
char *value;
1609-
size_t lensess;
1624+
char *p, *value;
16101625

16111626
switch (PS(session_status)) {
16121627
case php_session_active:
@@ -1648,8 +1663,6 @@ PHPAPI zend_result php_session_start(void) /* {{{ */
16481663
PS(send_cookie) = PS(use_cookies) || PS(use_only_cookies);
16491664
}
16501665

1651-
lensess = strlen(PS(session_name));
1652-
16531666
/*
16541667
* Cookies are preferred, because initially cookie and get
16551668
* variables will be available.
@@ -1661,7 +1674,7 @@ PHPAPI zend_result php_session_start(void) /* {{{ */
16611674
if (!PS(id)) {
16621675
if (PS(use_cookies) && (data = zend_hash_str_find(&EG(symbol_table), "_COOKIE", sizeof("_COOKIE") - 1))) {
16631676
ZVAL_DEREF(data);
1664-
if (Z_TYPE_P(data) == IS_ARRAY && (ppid = zend_hash_str_find(Z_ARRVAL_P(data), PS(session_name), lensess))) {
1677+
if (Z_TYPE_P(data) == IS_ARRAY && (ppid = zend_hash_find(Z_ARRVAL_P(data), PS(session_name)))) {
16651678
ppid2sid(ppid);
16661679
PS(send_cookie) = 0;
16671680
PS(define_sid) = 0;
@@ -1671,16 +1684,31 @@ PHPAPI zend_result php_session_start(void) /* {{{ */
16711684
if (!PS(use_only_cookies)) {
16721685
if (!PS(id) && (data = zend_hash_str_find(&EG(symbol_table), "_GET", sizeof("_GET") - 1))) {
16731686
ZVAL_DEREF(data);
1674-
if (Z_TYPE_P(data) == IS_ARRAY && (ppid = zend_hash_str_find(Z_ARRVAL_P(data), PS(session_name), lensess))) {
1687+
if (Z_TYPE_P(data) == IS_ARRAY && (ppid = zend_hash_find(Z_ARRVAL_P(data), PS(session_name)))) {
16751688
ppid2sid(ppid);
16761689
}
16771690
}
16781691
if (!PS(id) && (data = zend_hash_str_find(&EG(symbol_table), "_POST", sizeof("_POST") - 1))) {
16791692
ZVAL_DEREF(data);
1680-
if (Z_TYPE_P(data) == IS_ARRAY && (ppid = zend_hash_str_find(Z_ARRVAL_P(data), PS(session_name), lensess))) {
1693+
if (Z_TYPE_P(data) == IS_ARRAY && (ppid = zend_hash_find(Z_ARRVAL_P(data), PS(session_name)))) {
16811694
ppid2sid(ppid);
16821695
}
16831696
}
1697+
/* Check the REQUEST_URI symbol for a string of the form
1698+
* '<session-name>=<session-id>' to allow URLs of the form
1699+
* http://yoursite/<session-name>=<session-id>/script.php */
1700+
if (!PS(id) && zend_is_auto_global(ZSTR_KNOWN(ZEND_STR_AUTOGLOBAL_SERVER)) == SUCCESS &&
1701+
(data = zend_hash_str_find(Z_ARRVAL(PG(http_globals)[TRACK_VARS_SERVER]), "REQUEST_URI", sizeof("REQUEST_URI") - 1)) &&
1702+
Z_TYPE_P(data) == IS_STRING &&
1703+
(p = strstr(Z_STRVAL_P(data), ZSTR_VAL(PS(session_name)))) &&
1704+
p[ZSTR_LEN(PS(session_name))] == '='
1705+
) {
1706+
char *q;
1707+
p += ZSTR_LEN(PS(session_name));
1708+
if ((q = strpbrk(p, "/?\\"))) {
1709+
PS(id) = zend_string_init(p, q - p, 0);
1710+
}
1711+
}
16841712
/* Check whether the current request was referred to by
16851713
* an external site which invalidates the previously found id. */
16861714
if (PS(id) && PS(extern_referer_chk) && ZSTR_LEN(PS(extern_referer_chk)) != 0 &&
@@ -1763,7 +1791,7 @@ static zend_result php_session_reset(void) /* {{{ */
17631791
PHPAPI void session_adapt_url(const char *url, size_t url_len, char **new_url, size_t *new_len) /* {{{ */
17641792
{
17651793
if (APPLY_TRANS_SID && (PS(session_status) == php_session_active)) {
1766-
*new_url = php_url_scanner_adapt_single_url(url, url_len, PS(session_name), ZSTR_VAL(PS(id)), new_len, 1);
1794+
*new_url = php_url_scanner_adapt_single_url(url, url_len, ZSTR_VAL(PS(session_name)), ZSTR_VAL(PS(id)), new_len, 1);
17671795
}
17681796
}
17691797
/* }}} */
@@ -1985,7 +2013,8 @@ PHP_FUNCTION(session_name)
19852013
RETURN_FALSE;
19862014
}
19872015

1988-
RETVAL_STRING(PS(session_name));
2016+
// TODO Prevent duplication???
2017+
RETVAL_STR(zend_string_dup(PS(session_name), false));
19892018

19902019
if (name) {
19912020
ini_name = ZSTR_INIT_LITERAL("session.name", 0);
@@ -2403,7 +2432,7 @@ PHP_FUNCTION(session_regenerate_id)
24032432
zend_string_release_ex(PS(id), 0);
24042433
PS(id) = NULL;
24052434

2406-
if (PS(mod)->s_open(&PS(mod_data), PS(save_path), PS(session_name)) == FAILURE) {
2435+
if (PS(mod)->s_open(&PS(mod_data), PS(save_path), ZSTR_VAL(PS(session_name))) == FAILURE) {
24072436
PS(session_status) = php_session_none;
24082437
if (!EG(exception)) {
24092438
zend_throw_error(NULL, "Failed to open session: %s (path: %s)", PS(mod)->s_name, PS(save_path));
@@ -3117,7 +3146,7 @@ static bool early_find_sid_in(zval *dest, int where, php_session_rfc1867_progres
31173146
return 0;
31183147
}
31193148

3120-
if ((ppid = zend_hash_str_find(Z_ARRVAL(PG(http_globals)[where]), PS(session_name), progress->sname_len))
3149+
if ((ppid = zend_hash_find(Z_ARRVAL(PG(http_globals)[where]), PS(session_name)))
31213150
&& Z_TYPE_P(ppid) == IS_STRING) {
31223151
zval_ptr_dtor(dest);
31233152
ZVAL_COPY_DEREF(dest, ppid);
@@ -3225,7 +3254,8 @@ static zend_result php_session_rfc1867_callback(unsigned int event, void *event_
32253254
multipart_event_start *data = (multipart_event_start *) event_data;
32263255
progress = ecalloc(1, sizeof(php_session_rfc1867_progress));
32273256
progress->content_length = data->content_length;
3228-
progress->sname_len = strlen(PS(session_name));
3257+
// TODO Remove field?
3258+
progress->sname_len = ZSTR_LEN(PS(session_name));
32293259
PS(rfc1867_progress) = progress;
32303260
}
32313261
break;
@@ -3247,7 +3277,7 @@ static zend_result php_session_rfc1867_callback(unsigned int event, void *event_
32473277
if (data->name && data->value && value_len) {
32483278
size_t name_len = strlen(data->name);
32493279

3250-
if (name_len == progress->sname_len && memcmp(data->name, PS(session_name), name_len) == 0) {
3280+
if (name_len == progress->sname_len && memcmp(data->name, ZSTR_VAL(PS(session_name)), name_len) == 0) {
32513281
zval_ptr_dtor(&progress->sid);
32523282
ZVAL_STRINGL(&progress->sid, (*data->value), value_len);
32533283
} 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
@@ -48,7 +48,7 @@ var_dump($_SESSION["upload_progress_" . basename(__FILE__)]);
4848
session_destroy();
4949
?>
5050
--EXPECTF--
51-
string(%d) "rfc1867-sid-post"
51+
string(16) "rfc1867-sid-post"
5252
bool(true)
5353
array(2) {
5454
["file1"]=>

‎ext/session/tests/session_name_variation1.phpt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ ob_end_flush();
3232
?>
3333
--EXPECTF--
3434
*** Testing session_name() : variation ***
35+
36+
Warning: session_name(): session.name "" cannot contain nul bytes in %s on line %d
3537
string(9) "PHPSESSID"
3638

3739
Warning: session_start(): session.name cannot contain any of the following '=,;.[ \t\r\n\013\014' in %s on line %d
@@ -40,7 +42,7 @@ string(1) " "
4042
bool(true)
4143
string(1) " "
4244

43-
Warning: session_name(): session.name "" cannot be numeric or empty in %s on line %d
45+
Warning: session_name(): session.name "" cannot be empty in %s on line %d
4446
string(1) " "
4547

4648
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)