Skip to content

Commit 6f73a0c

Browse files
committed
Merge branch 'pr368' into PHP-5.5
* pr368: fix crash, enable session_id and fix test Strict session. Detect session id collision Strict session
2 parents 7d3fa7d + b80d73c commit 6f73a0c

39 files changed

+264
-95
lines changed

Diff for: ext/session/mod_files.c

+60-34
Original file line numberDiff line numberDiff line change
@@ -61,40 +61,9 @@ typedef struct {
6161
} ps_files;
6262

6363
ps_module ps_mod_files = {
64-
PS_MOD(files)
64+
PS_MOD_SID(files)
6565
};
6666

67-
/* If you change the logic here, please also update the error message in
68-
* ps_files_open() appropriately */
69-
static int ps_files_valid_key(const char *key)
70-
{
71-
size_t len;
72-
const char *p;
73-
char c;
74-
int ret = 1;
75-
76-
for (p = key; (c = *p); p++) {
77-
/* valid characters are a..z,A..Z,0..9 */
78-
if (!((c >= 'a' && c <= 'z')
79-
|| (c >= 'A' && c <= 'Z')
80-
|| (c >= '0' && c <= '9')
81-
|| c == ','
82-
|| c == '-')) {
83-
ret = 0;
84-
break;
85-
}
86-
}
87-
88-
len = p - key;
89-
90-
/* Somewhat arbitrary length limit here, but should be way more than
91-
anyone needs and avoids file-level warnings later on if we exceed MAX_PATH */
92-
if (len == 0 || len > 128) {
93-
ret = 0;
94-
}
95-
96-
return ret;
97-
}
9867

9968
static char *ps_files_path_create(char *buf, size_t buflen, ps_files *data, const char *key)
10069
{
@@ -155,11 +124,11 @@ static void ps_files_open(ps_files *data, const char *key TSRMLS_DC)
155124

156125
ps_files_close(data);
157126

158-
if (!ps_files_valid_key(key)) {
127+
if (php_session_valid_key(key) == FAILURE) {
159128
php_error_docref(NULL TSRMLS_CC, E_WARNING, "The session id is too long or contains illegal characters, valid characters are a-z, A-Z, 0-9 and '-,'");
160-
PS(invalid_session_id) = 1;
161129
return;
162130
}
131+
163132
if (!ps_files_path_create(buf, sizeof(buf), data, key)) {
164133
return;
165134
}
@@ -253,6 +222,21 @@ static int ps_files_cleanup_dir(const char *dirname, int maxlifetime TSRMLS_DC)
253222
return (nrdels);
254223
}
255224

225+
static int ps_files_key_exists(ps_files *data, const char *key TSRMLS_DC)
226+
{
227+
char buf[MAXPATHLEN];
228+
struct stat sbuf;
229+
230+
if (!key || !ps_files_path_create(buf, sizeof(buf), data, key)) {
231+
return FAILURE;
232+
}
233+
if (VCWD_STAT(buf, &sbuf)) {
234+
return FAILURE;
235+
}
236+
return SUCCESS;
237+
}
238+
239+
256240
#define PS_FILES_DATA ps_files *data = PS_GET_MOD_DATA()
257241

258242
PS_OPEN_FUNC(files)
@@ -342,6 +326,24 @@ PS_READ_FUNC(files)
342326
struct stat sbuf;
343327
PS_FILES_DATA;
344328

329+
/* If strict mode, check session id existence */
330+
if (PS(use_strict_mode) &&
331+
ps_files_key_exists(data, key TSRMLS_CC) == FAILURE) {
332+
/* key points to PS(id), but cannot change here. */
333+
if (key) {
334+
efree(PS(id));
335+
PS(id) = NULL;
336+
}
337+
PS(id) = PS(mod)->s_create_sid((void **)&data, NULL TSRMLS_CC);
338+
if (!PS(id)) {
339+
return FAILURE;
340+
}
341+
php_session_reset_id(TSRMLS_C);
342+
if (PS(use_cookies)) {
343+
PS(send_cookie) = 1;
344+
}
345+
}
346+
345347
ps_files_open(data, key TSRMLS_CC);
346348
if (data->fd < 0) {
347349
return FAILURE;
@@ -454,6 +456,30 @@ PS_GC_FUNC(files)
454456
return SUCCESS;
455457
}
456458

459+
PS_CREATE_SID_FUNC(files)
460+
{
461+
char *sid;
462+
int maxfail = 3;
463+
PS_FILES_DATA;
464+
465+
do {
466+
sid = php_session_create_id((void **)&data, newlen TSRMLS_CC);
467+
/* Check collision */
468+
if (data && ps_files_key_exists(data, sid TSRMLS_CC) == SUCCESS) {
469+
if (sid) {
470+
efree(sid);
471+
sid = NULL;
472+
}
473+
if (!(maxfail--)) {
474+
return NULL;
475+
}
476+
}
477+
} while(!sid);
478+
479+
return sid;
480+
}
481+
482+
457483
/*
458484
* Local variables:
459485
* tab-width: 4

Diff for: ext/session/mod_files.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,6 @@
2424
extern ps_module ps_mod_files;
2525
#define ps_files_ptr &ps_mod_files
2626

27-
PS_FUNCS(files);
27+
PS_FUNCS_SID(files);
2828

2929
#endif

Diff for: ext/session/mod_mm.c

+57-2
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ static ps_sd *ps_sd_new(ps_mm *data, const char *key)
124124
if (!sd) {
125125
TSRMLS_FETCH();
126126

127-
php_error_docref(NULL TSRMLS_CC, E_WARNING, "mm_malloc failed, avail %d, err %s", mm_available(data->mm), mm_error());
127+
php_error_docref(NULL TSRMLS_CC, E_WARNING, "mm_malloc failed, avail %ld, err %s", mm_available(data->mm), mm_error());
128128
return NULL;
129129
}
130130

@@ -208,8 +208,22 @@ static ps_sd *ps_sd_lookup(ps_mm *data, const char *key, int rw)
208208
return ret;
209209
}
210210

211+
static int ps_mm_key_exists(ps_mm *data, const char *key TSRMLS_DC)
212+
{
213+
ps_sd *sd;
214+
215+
if (!key) {
216+
return FAILURE;
217+
}
218+
sd = ps_sd_lookup(data, key, 0);
219+
if (sd) {
220+
return SUCCESS;
221+
}
222+
return FAILURE;
223+
}
224+
211225
ps_module ps_mod_mm = {
212-
PS_MOD(mm)
226+
PS_MOD_SID(mm)
213227
};
214228

215229
#define PS_MM_DATA ps_mm *data = PS_GET_MOD_DATA()
@@ -341,6 +355,24 @@ PS_READ_FUNC(mm)
341355

342356
mm_lock(data->mm, MM_LOCK_RD);
343357

358+
/* If there is an ID and strict mode, verify existence */
359+
if (PS(use_strict_mode)
360+
&& ps_mm_key_exists(data, key TSRMLS_CC) == FAILURE) {
361+
/* key points to PS(id), but cannot change here. */
362+
if (key) {
363+
efree(PS(id));
364+
PS(id) = NULL;
365+
}
366+
PS(id) = PS(mod)->s_create_sid((void **)&data, NULL TSRMLS_CC);
367+
if (!PS(id)) {
368+
return FAILURE;
369+
}
370+
php_session_reset_id(TSRMLS_C);
371+
if (PS(use_cookies)) {
372+
PS(send_cookie) = 1;
373+
}
374+
}
375+
344376
sd = ps_sd_lookup(data, key, 0);
345377
if (sd) {
346378
*vallen = sd->datalen;
@@ -444,6 +476,29 @@ PS_GC_FUNC(mm)
444476
return SUCCESS;
445477
}
446478

479+
PS_CREATE_SID_FUNC(mm)
480+
{
481+
char *sid;
482+
int maxfail = 3;
483+
PS_MM_DATA;
484+
485+
do {
486+
sid = php_session_create_id((void **)&data, newlen TSRMLS_CC);
487+
/* Check collision */
488+
if (ps_mm_key_exists(data, sid TSRMLS_CC) == SUCCESS) {
489+
if (sid) {
490+
efree(sid);
491+
sid = NULL;
492+
}
493+
if (!(maxfail--)) {
494+
return NULL;
495+
}
496+
}
497+
} while(!sid);
498+
499+
return sid;
500+
}
501+
447502
#endif
448503

449504
/*

Diff for: ext/session/php_session.h

+9-1
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@
2929

3030
#define PHP_SESSION_API 20020330
3131

32+
/* To check php_session_valid_key()/php_session_reset_id() */
33+
#define PHP_SESSION_STRICT 1
34+
3235
#define PS_OPEN_ARGS void **mod_data, const char *save_path, const char *session_name TSRMLS_DC
3336
#define PS_CLOSE_ARGS void **mod_data TSRMLS_DC
3437
#define PS_READ_ARGS void **mod_data, const char *key, char **val, int *vallen TSRMLS_DC
@@ -75,7 +78,7 @@ typedef struct ps_module_struct {
7578
#x, ps_open_##x, ps_close_##x, ps_read_##x, ps_write_##x, \
7679
ps_delete_##x, ps_gc_##x, php_session_create_id
7780

78-
/* SID enabled module handler definitions */
81+
/* SID creation enabled module handler definitions */
7982
#define PS_FUNCS_SID(x) \
8083
PS_OPEN_FUNC(x); \
8184
PS_CLOSE_FUNC(x); \
@@ -175,6 +178,8 @@ typedef struct _php_ps_globals {
175178
smart_str rfc1867_name; /* session.upload_progress.name */
176179
long rfc1867_freq; /* session.upload_progress.freq */
177180
double rfc1867_min_freq; /* session.upload_progress.min_freq */
181+
182+
zend_bool use_strict_mode; /* whether or not PHP accepts unknown session ids */
178183
} php_ps_globals;
179184

180185
typedef php_ps_globals zend_ps_globals;
@@ -230,6 +235,9 @@ PHPAPI void php_session_start(TSRMLS_D);
230235
PHPAPI ps_module *_php_find_ps_module(char *name TSRMLS_DC);
231236
PHPAPI const ps_serializer *_php_find_ps_serializer(char *name TSRMLS_DC);
232237

238+
PHPAPI int php_session_valid_key(const char *key);
239+
PHPAPI void php_session_reset_id(TSRMLS_D);
240+
233241
#define PS_ADD_VARL(name,namelen) do { \
234242
php_add_session_var(name, namelen TSRMLS_CC); \
235243
} while (0)

0 commit comments

Comments
 (0)