Skip to content

Commit e3133e4

Browse files
committed
Fix bug #77630 - safer rename() procedure
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
1 parent e0f5d62 commit e3133e4

File tree

1 file changed

+34
-17
lines changed

1 file changed

+34
-17
lines changed

main/streams/plain_wrapper.c

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1168,34 +1168,51 @@ static int php_plain_files_rename(php_stream_wrapper *wrapper, const char *url_f
11681168
# ifdef EXDEV
11691169
if (errno == EXDEV) {
11701170
zend_stat_t sb;
1171+
# if !defined(ZTS) && !defined(TSRM_WIN32) && !defined(NETWARE)
1172+
/* not sure what to do in ZTS case, umask is not thread-safe */
1173+
int oldmask = umask(077);
1174+
# endif
1175+
int success = 0;
11711176
if (php_copy_file(url_from, url_to) == SUCCESS) {
11721177
if (VCWD_STAT(url_from, &sb) == 0) {
1178+
success = 1;
11731179
# if !defined(TSRM_WIN32) && !defined(NETWARE)
1174-
if (VCWD_CHMOD(url_to, sb.st_mode)) {
1175-
if (errno == EPERM) {
1176-
php_error_docref2(NULL, url_from, url_to, E_WARNING, "%s", strerror(errno));
1177-
VCWD_UNLINK(url_from);
1178-
return 1;
1179-
}
1180+
/*
1181+
* Try to set user and permission info on the target.
1182+
* If we're not root, then some of these may fail.
1183+
* We try chown first, to set proper group info, relying
1184+
* on the system environment to have proper umask to not allow
1185+
* access to the file in the meantime.
1186+
*/
1187+
if (VCWD_CHOWN(url_to, sb.st_uid, sb.st_gid)) {
11801188
php_error_docref2(NULL, url_from, url_to, E_WARNING, "%s", strerror(errno));
1181-
return 0;
1189+
if (errno != EPERM) {
1190+
success = 0;
1191+
}
11821192
}
1183-
if (VCWD_CHOWN(url_to, sb.st_uid, sb.st_gid)) {
1184-
if (errno == EPERM) {
1193+
1194+
if (success) {
1195+
if (VCWD_CHMOD(url_to, sb.st_mode)) {
11851196
php_error_docref2(NULL, url_from, url_to, E_WARNING, "%s", strerror(errno));
1186-
VCWD_UNLINK(url_from);
1187-
return 1;
1197+
if (errno != EPERM) {
1198+
success = 0;
1199+
}
11881200
}
1189-
php_error_docref2(NULL, url_from, url_to, E_WARNING, "%s", strerror(errno));
1190-
return 0;
11911201
}
11921202
# endif
1193-
VCWD_UNLINK(url_from);
1194-
return 1;
1203+
if (success) {
1204+
VCWD_UNLINK(url_from);
1205+
}
1206+
} else {
1207+
php_error_docref2(NULL, url_from, url_to, E_WARNING, "%s", strerror(errno));
11951208
}
1209+
} else {
1210+
php_error_docref2(NULL, url_from, url_to, E_WARNING, "%s", strerror(errno));
11961211
}
1197-
php_error_docref2(NULL, url_from, url_to, E_WARNING, "%s", strerror(errno));
1198-
return 0;
1212+
# if !defined(ZTS) && !defined(TSRM_WIN32) && !defined(NETWARE)
1213+
umask(oldmask);
1214+
# endif
1215+
return success;
11991216
}
12001217
# endif
12011218
#endif

0 commit comments

Comments
 (0)