Skip to content

Commit

Permalink
Fix bug #77630 - safer rename() procedure
Browse files Browse the repository at this point in the history
In order to rename safer, we do the following:
- set umask to 077 (unfortunately, not TS, so excluding ZTS)
- chown() first, to set proper group before allowing group access
- chmod() after, even if chown() fails
  • Loading branch information
smalyshev committed Mar 4, 2019
1 parent e0f5d62 commit e3133e4
Showing 1 changed file with 34 additions and 17 deletions.
51 changes: 34 additions & 17 deletions main/streams/plain_wrapper.c
Original file line number Diff line number Diff line change
Expand Up @@ -1168,34 +1168,51 @@ static int php_plain_files_rename(php_stream_wrapper *wrapper, const char *url_f
# ifdef EXDEV
if (errno == EXDEV) {
zend_stat_t sb;
# if !defined(ZTS) && !defined(TSRM_WIN32) && !defined(NETWARE)
/* not sure what to do in ZTS case, umask is not thread-safe */
int oldmask = umask(077);
# endif
int success = 0;
if (php_copy_file(url_from, url_to) == SUCCESS) {
if (VCWD_STAT(url_from, &sb) == 0) {
success = 1;
# if !defined(TSRM_WIN32) && !defined(NETWARE)
if (VCWD_CHMOD(url_to, sb.st_mode)) {
if (errno == EPERM) {
php_error_docref2(NULL, url_from, url_to, E_WARNING, "%s", strerror(errno));
VCWD_UNLINK(url_from);
return 1;
}
/*
* Try to set user and permission info on the target.
* If we're not root, then some of these may fail.
* We try chown first, to set proper group info, relying
* on the system environment to have proper umask to not allow
* access to the file in the meantime.
*/
if (VCWD_CHOWN(url_to, sb.st_uid, sb.st_gid)) {
php_error_docref2(NULL, url_from, url_to, E_WARNING, "%s", strerror(errno));
return 0;
if (errno != EPERM) {
success = 0;
}
}
if (VCWD_CHOWN(url_to, sb.st_uid, sb.st_gid)) {
if (errno == EPERM) {

if (success) {
if (VCWD_CHMOD(url_to, sb.st_mode)) {
php_error_docref2(NULL, url_from, url_to, E_WARNING, "%s", strerror(errno));
VCWD_UNLINK(url_from);
return 1;
if (errno != EPERM) {
success = 0;
}
}
php_error_docref2(NULL, url_from, url_to, E_WARNING, "%s", strerror(errno));
return 0;
}
# endif
VCWD_UNLINK(url_from);
return 1;
if (success) {
VCWD_UNLINK(url_from);
}
} else {
php_error_docref2(NULL, url_from, url_to, E_WARNING, "%s", strerror(errno));
}
} else {
php_error_docref2(NULL, url_from, url_to, E_WARNING, "%s", strerror(errno));
}
php_error_docref2(NULL, url_from, url_to, E_WARNING, "%s", strerror(errno));
return 0;
# if !defined(ZTS) && !defined(TSRM_WIN32) && !defined(NETWARE)
umask(oldmask);
# endif
return success;
}
# endif
#endif
Expand Down

0 comments on commit e3133e4

Please sign in to comment.