[wp-trac] [WordPress Trac] #42026: Regression in 4.8.2: WordPress now prevents certain unzipping valid filename patterns (but not creating them)
WordPress Trac
noreply at wordpress.org
Thu Sep 28 20:25:13 UTC 2017
#42026: Regression in 4.8.2: WordPress now prevents certain unzipping valid
filename patterns (but not creating them)
---------------------------+-----------------------------
Reporter: DavidAnderson | Owner:
Type: defect (bug) | Status: new
Priority: normal | Milestone: Awaiting Review
Component: General | Version: 4.8.2
Severity: normal | Keywords:
Focuses: |
---------------------------+-----------------------------
WordPress 4.8.2 added a new check into both the `_unzip_file_pclzip()` and
`_unzip_file_ziparchive()` methods:
{{{#!php
if ( 0 !== validate_file( $info['name'] ) ) {
return new WP_Error( 'invalid_file_ziparchive',
__( 'Could not extract file from archive.' ), $info['name'] );
}
}}}
To return 0, `validate_file()` requires, among other things, that the
filename cannot include two consecutive periods. The aim here is
presumably to prevent directory traversal attacks. Unfortunately, the
implementation is naive and includes much more than this.
As a result, WordPress will allow you to upload files (e.g. into the media
library) with names like `picture..jpg`, but not to unzip them.
My particular problem here is that this prevents the restoration of
backups. (I am the lead developer of UpdraftPlus, the #1 most-installed WP
backup/restore plugin - over 1 million active installs). If your media
library includes files with patterns like this (and we've already
encountered multiple users who do), then you can no longer restore the
backup. There's no filter provided in either of the unzip functions, or in
`validate_file()`, to get around this. The only work-around is to clone
large chunks of WP's zip-handling API (from
https://codex.wordpress.org/Function_Reference/unzip_file downwards) in
order to alter one line, to get back the old behaviour.
In my view the check in `validate_file()` is the problem - forbidding two
consecutive periods is too crude. If they're going to be forbidden when
unzipping, they should be forbidden everywhere else too (e.g.
uploading)... but this should not be enforced on existing WP installs
which already have such existing content. Another satisfactory solution
would be to allow the result of `validate_file()` to be filtered so that
particular consumes of `unzip_file()` can apply their own checks.
--
Ticket URL: <https://core.trac.wordpress.org/ticket/42026>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list