diff options
author | Stanislav Malyshev <stas@php.net> | 2014-04-13 20:31:20 -0700 |
---|---|---|
committer | Stanislav Malyshev <stas@php.net> | 2014-04-14 10:44:53 -0700 |
commit | 40a9316dff6e043b534844b2ab167318250be277 (patch) | |
tree | 5c1c42a8a8aa07b5bb455613b411c84116e6905d | |
parent | 4268504084868b56ac8632aeca731eea2a4c7955 (diff) | |
download | php-git-40a9316dff6e043b534844b2ab167318250be277.tar.gz |
Fix bug #66171: better handling of symlinks
-rw-r--r-- | NEWS | 2 | ||||
-rw-r--r-- | ext/session/mod_files.c | 37 |
2 files changed, 22 insertions, 17 deletions
@@ -8,6 +8,8 @@ PHP NEWS UNIX sockets). (Mike) . Fixed bug #64604 (parse_url is inconsistent with specified port). (Ingo Walz) + . Fixed bug #66171 (Symlinks and session handler allow open_basedir bypass). + (Jann Horn, Stas) . Fixed bug #66182 (exit in stream filter produces segfault). (Mike) . Fixed bug #66736 (fpassthru broken). (Mike) . Fixed bug #67024 (getimagesize should recognize BMP files with negative diff --git a/ext/session/mod_files.c b/ext/session/mod_files.c index 86a2235845..8f57ca5af9 100644 --- a/ext/session/mod_files.c +++ b/ext/session/mod_files.c @@ -146,6 +146,7 @@ static void ps_files_close(ps_files *data) static void ps_files_open(ps_files *data, const char *key TSRMLS_DC) { char buf[MAXPATHLEN]; + struct stat sbuf; if (data->fd < 0 || !data->lastkey || strcmp(key, data->lastkey)) { if (data->lastkey) { @@ -165,26 +166,28 @@ static void ps_files_open(ps_files *data, const char *key TSRMLS_DC) } data->lastkey = estrdup(key); - - data->fd = VCWD_OPEN_MODE(buf, O_CREAT | O_RDWR | O_BINARY, data->filemode); + + /* O_NOFOLLOW to prevent us from following evil symlinks */ +#ifdef O_NOFOLLOW + data->fd = VCWD_OPEN_MODE(buf, O_CREAT | O_RDWR | O_BINARY | O_NOFOLLOW, data->filemode); +#else + /* Check to make sure that the opened file is not outside of allowable dirs. + This is not 100% safe but it's hard to do something better without O_NOFOLLOW */ + if(PG(open_basedir) && lstat(buf, &sbuf) == 0 && S_ISLNK(sbuf.st_mode) && php_check_open_basedir(buf TSRMLS_CC)) { + return; + } + data->fd = VCWD_OPEN_MODE(buf, O_CREAT | O_RDWR | O_BINARY, data->filemode); +#endif if (data->fd != -1) { #ifndef PHP_WIN32 - /* check to make sure that the opened file is not a symlink, linking to data outside of allowable dirs */ - if (PG(open_basedir)) { - struct stat sbuf; - - if (fstat(data->fd, &sbuf)) { - close(data->fd); - data->fd = -1; - return; - } - if (S_ISLNK(sbuf.st_mode) && php_check_open_basedir(buf TSRMLS_CC)) { - close(data->fd); - data->fd = -1; - return; - } - } + /* check that this session file was created by us or root – we + don't want to end up accepting the sessions of another webapp */ + if (fstat(data->fd, &sbuf) || (sbuf.st_uid != 0 && sbuf.st_uid != getuid() && sbuf.st_uid != geteuid())) { + close(data->fd); + data->fd = -1; + return; + } #endif flock(data->fd, LOCK_EX); |