Opened 9 years ago
Closed 9 years ago
#34562 closed defect (bug) (fixed)
`wp_tempnam()` can return a path that already exists
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.4 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Filesystem API | Keywords: | has-patch |
Focuses: | Cc: |
Description
At present it's possible for the wp_tempnam()
to generate a filename which already exists on the server, it requires two processes to attempt to create the same filename within the same short period of time. The period of time varies on the IO operation time, so is more of an issue where the filesystem is remote (say a NFS share).
There's two things we can do to improve this
a) Suffix a random ID onto the filename to make it far more unique, and unlikely to conflict with an existing file
b) verify the result from touch()
to see if the file exists
a
uses wp_generate_password( 6, $special_char=false )
which is the only real random string function we have - uniq()
is possible but is mearly the same as microtime()
converted to a-Z.
b
is actually a little complex, touch()
isn't what we want here, what we want is more akin to fopen( $file, 'x' )
- create the file if it doesn't exist, else bail.
Attached is a patch that implements this with a few extra checks, it currently allows for an unwritable directory/filename to be returned (I mean, it's best-effort, the failure isn't likely to occur) to prevent infinite looping.
Attachments (1)
Change History (5)
#3
@
9 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
This unfortunately didn't get applied to the WP_Upgrader class too, which doesn't rely upon wp_tempnam()
as it's after a directory name, not a filename.
We should apply something similar to Upgrades to reduce the chances of issues caused by any upgrade-related race conditions. 34562.2.diff should do this.
In 35644: