Bug #55576 [Opn->Csd]: Race condition in move_uploaded_file()

Edit report at https://bugs.php.net/bug.php?id=55576&edit=1 ID: 55576 Updated by: cataphract@php.net Reported by: cjk at wwwtech dot de Summary: Race condition in move_uploaded_file() -Status: Open +Status: Closed Type: Bug Package: Filesystem function related Operating System: All PHP Version: 5.3.8 -Assigned To: +Assigned To: cataphract Block user comment: N Private report: N New Comment: Fixed by removing unlink of destination file. Thank you for your report. Previous Comments: ------------------------------------------------------------------------ [2011-09-04 23:00:35] cataphract@php.net Automatic comment from SVN on behalf of cataphract Revision: http://svn.php.net/viewvc/?view=revision&revision=316122 Log: - Fixed bug #55576: Cannot conditionally move uploaded file without race condition. ------------------------------------------------------------------------ [2011-09-04 10:57:58] cjk at wwwtech dot de Removing the unlink() would at least give us the possibility to make a file upload concurrency safe when using move_uploaded_file() ------------------------------------------------------------------------ [2011-09-03 18:16:02] cataphract@php.net The patch makes sense for paths in the filesystem, but this function also supports an arbitrary stream wrapper in the destination. In any case, I'm puzzled by the first unlink() call (on new_path), it seems redundant. It was introduced in r32313. ------------------------------------------------------------------------ [2011-09-03 11:34:19] cjk at wwwtech dot de Description: ------------ There is a race condition in the move_uploaded_file() function: if you don't want to overwrite a file, the standard mechanism is: $fd = fopen($file,"x"); fclose($fd); move_uploaded_file($uploaded_file,$file); But since move_uploaded_file() unlink()s a file first, there may be a race condition: file gets created exclusively via fopen(…,"x"), move_uploaded_file() removes the same file and the process gets suspended. Another process creates the file via fopen(…,"x"), voila, race condition. Expected result: ---------------- We need a concurrency save implementation of move_uploaded_file(). This can be achieved by implementing a third parameter, boolean $dont_overwrite. When set to true, move_uploaded_file() will ensure that the file does not exist by using open(…,O_RDWR|O_CREAT|O_EXCL) and returning false in error case. The patch I attached does exactly this. Actual result: -------------- When two concurrent processes, they may overwrite the same file twice w/o the possibility to prevent it. ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=55576&edit=1