Opened 15 years ago
Last modified 2 years ago
#12756 reopened defect (bug)
WPMU does not handle files with two or more dots in the filename
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | minor | Version: | 2.9.2 |
Component: | Upload | Keywords: | has-patch dev-feedback close |
Focuses: | multisite | Cc: |
Description
- WPMU does download images that have two or more dots in the file name
E.g., One..jpg One...jpg One....jpg
rewrites do work (checked)
- this is clearly a WP issue:
/wp-content/blogs.php
...
$file = BLOGUPLOADDIR . str_replace( '..', , $_GET[ 'file' ] );
if ( !is_file( $file ) ) {
status_header( 404 );
die('404 — File not found.');
}
...
WPMU removes two dots!!!
workaround:
$file = BLOGUPLOADDIR . $_GET[ 'file' ]; name.ly: workaround for files with two or more dots
tested and works fine
Attachments (1)
Change History (20)
#5
@
14 years ago
The purpose of the str_replace was to prevent a request like /files/../../../../wp-config.php
from being processed.
Probably $file = BLOGUPLOADDIR . str_replace( '../', , $_GET[ 'file' ] );
would work, but I haven't tested it.
#6
@
14 years ago
I seem to be missing two quotes there
$file = BLOGUPLOADDIR . str_replace( '../', '', $_GET[ 'file' ] );
#7
@
14 years ago
- Cc larysa added
- Owner set to wpmuguru
- Status changed from new to assigned
Anything
$file = BLOGUPLOADDIR . str_replace( '../', , $_GET[ 'file' ] );
or
$file = BLOGUPLOADDIR . str_replace( '/..', , $_GET[ 'file' ] );
or
$file = BLOGUPLOADDIR . str_replace( '/../', , $_GET[ 'file' ] );
would solve the issue.
The last one is enough. Any chance to see this change in the official release soon? Shame to miss it in 3.0.2.
#8
@
14 years ago
- Keywords has-patch added
I think we should 404 the request if it has a '../' in the request uri.
#9
@
14 years ago
- Keywords needs-patch added
- Version changed from 2.9.2 to 3.2.1
We are using:
$file = BLOGUPLOADDIR . str_replace( '../', '', $_GET[ 'file' ] );
If would be nice to have it in the next release, not to hack the code each time we run an upgrade.
Thank you.
#10
@
14 years ago
- Keywords needs-patch removed
- Version changed from 3.2.1 to 2.9.2
Please do not change the version number. It's there to track when the bug was introduced/reported.
Also, the patch still looks valid and applies correctly.
#12
follow-up:
↓ 13
@
13 years ago
- Milestone changed from 3.3 to Future Release
Using validate_file() here would yield the same bug anyway. Curious if ".." rather than "../" could therefore prevent path traversal with "..\". Since this isn't a regression, I'm inclined to continue to punt it.
In IRC, we're wondering whether a backslash could cause some issues, either via traversal or by escaping characters.
The simple fix is to just not use ..
in a URL. Punt.
#13
in reply to:
↑ 12
@
13 years ago
Replying to nacin:
In IRC, we're wondering whether a backslash could cause some issues, either via traversal or by escaping characters.
The simple fix is to just not use
..
in a URL. Punt.
For this one, I think the better solution is to sanitize the filename on upload and replace the .. with --.
#14
@
12 years ago
- Keywords close added
This was more or less addressed by #19235. Recommending close.
#15
@
12 years ago
- Keywords close removed
- Milestone Future Release deleted
- Resolution set to invalid
- Status changed from assigned to closed
#16
@
12 years ago
- Milestone set to Future Release
- Resolution invalid deleted
- Status changed from closed to reopened
Unfortunately, people will be running ms-files.php for the foreseeable future. Unlike global terms, moving away from ms-files.php is not easy. Some fixes to ms-files like this can be easy wins.
#17
@
10 years ago
WordPress 4.1 multisite still allow to upload files with two dots but don't allow to download these files:
http://example.lt/multisite/userpage/files/2015/01/test..pdf 404 — File not found.
Errata, yes WPMU does download the files into the blogs.dir... but /wp-content/blogs.php does not handle them properly, hence, the files are not seen by the user.
Needs to be corrected.