Skip to content

Commit 7f43aeb

Browse files
committed
Fix bug #66171: better handling of symlinks
1 parent f6db105 commit 7f43aeb

File tree

1 file changed

+20
-17
lines changed

1 file changed

+20
-17
lines changed

ext/session/mod_files.c

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ static void ps_files_close(ps_files *data)
115115
static void ps_files_open(ps_files *data, const char *key TSRMLS_DC)
116116
{
117117
char buf[MAXPATHLEN];
118+
struct stat sbuf;
118119

119120
if (data->fd < 0 || !data->lastkey || strcmp(key, data->lastkey)) {
120121
if (data->lastkey) {
@@ -134,26 +135,28 @@ static void ps_files_open(ps_files *data, const char *key TSRMLS_DC)
134135
}
135136

136137
data->lastkey = estrdup(key);
137-
138-
data->fd = VCWD_OPEN_MODE(buf, O_CREAT | O_RDWR | O_BINARY, data->filemode);
138+
139+
/* O_NOFOLLOW to prevent us from following evil symlinks */
140+
#ifdef O_NOFOLLOW
141+
data->fd = VCWD_OPEN_MODE(buf, O_CREAT | O_RDWR | O_BINARY | O_NOFOLLOW, data->filemode);
142+
#else
143+
/* Check to make sure that the opened file is not outside of allowable dirs.
144+
This is not 100% safe but it's hard to do something better without O_NOFOLLOW */
145+
if(PG(open_basedir) && lstat(buf, &sbuf) == 0 && S_ISLNK(sbuf.st_mode) && php_check_open_basedir(buf TSRMLS_CC)) {
146+
return;
147+
}
148+
data->fd = VCWD_OPEN_MODE(buf, O_CREAT | O_RDWR | O_BINARY, data->filemode);
149+
#endif
139150

140151
if (data->fd != -1) {
141152
#ifndef PHP_WIN32
142-
/* check to make sure that the opened file is not a symlink, linking to data outside of allowable dirs */
143-
if (PG(open_basedir)) {
144-
struct stat sbuf;
145-
146-
if (fstat(data->fd, &sbuf)) {
147-
close(data->fd);
148-
data->fd = -1;
149-
return;
150-
}
151-
if (S_ISLNK(sbuf.st_mode) && php_check_open_basedir(buf TSRMLS_CC)) {
152-
close(data->fd);
153-
data->fd = -1;
154-
return;
155-
}
156-
}
153+
/* check that this session file was created by us or root – we
154+
don't want to end up accepting the sessions of another webapp */
155+
if (fstat(data->fd, &sbuf) || (sbuf.st_uid != 0 && sbuf.st_uid != getuid() && sbuf.st_uid != geteuid())) {
156+
close(data->fd);
157+
data->fd = -1;
158+
return;
159+
}
157160
#endif
158161
flock(data->fd, LOCK_EX);
159162

0 commit comments

Comments
 (0)