sergiotarxz
/
Wordpress
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Filesystem API: Add more specificity to the rules for valid files in `validate_file()`. 

This now treats files containing `./` as valid, and also treats files containing a trailing `../` as valid due to widespread use of this pattern in theme and plugin zip files.

Adds tests.

Props Ipstenu, borgesbruno, DavidAnderson, philipjohn, birgire
Fixes #42016, #36170


git-svn-id: https://develop.svn.wordpress.org/trunk@42011 602fd350-edb4-49c9-b593-d223f7449a82
This commit is contained in:
John Blackbourn 2017-10-24 23:14:33 +00:00
parent 49b7cb458f
commit 3e9a42ed27
3 changed files with 158 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/wp-admin/includes/file.php
View File

@ -663,7 +663,7 @@ function wp_tempnam( $filename = '', $dir = '' ) {
* @param array $allowed_files Optional. Array of allowed files to edit, $file must match an entry exactly.
* @return string|null
*/
function validate_file_to_edit( $file, $allowed_files = '' ) {
function validate_file_to_edit( $file, $allowed_files = array() ) {
$code = validate_file( $file, $allowed_files );
if (!$code )

17
src/wp-includes/functions.php
View File

@ -4252,16 +4252,27 @@ function iis7_supports_permalinks() {
* @param array $allowed_files Optional. List of allowed files.
* @return int 0 means nothing is wrong, greater than 0 means something was wrong.
*/
function validate_file( $file, $allowed_files = '' ) {
if ( false !== strpos( $file, '..' ) )
function validate_file( $file, $allowed_files = array() ) {
// `../` on its own is not allowed:
if ( '../' === $file ) {
return 1;
}
if ( false !== strpos( $file, './' ) )
// More than one occurence of `../` is not allowed:
if ( preg_match_all( '#\.\./#', $file, $matches, PREG_SET_ORDER ) && ( count( $matches ) > 1 ) ) {
return 1;
}
// `../` which does not occur at the end of the path is not allowed:
if ( false !== strpos( $file, '../' ) && '../' !== mb_substr( $file, -3, 3 ) ) {
return 1;
}
// Files not in the allowed file list are not allowed:
if ( ! empty( $allowed_files ) && ! in_array( $file, $allowed_files ) )
return 3;
// Absolute Windows drive paths are not allowed:
if (':' == substr( $file, 1, 1 ) )
return 2;

143
tests/phpunit/tests/functions.php
View File

@ -1161,4 +1161,147 @@ class Tests_Functions extends WP_UnitTestCase {
return $data;
}
/**
* Test file path validation
*
* @ticket 42016
* @dataProvider data_test_validate_file()
*
* @param string $file File path.
* @param array $allowed_files List of allowed files.
* @param int $expected Expected result.
*/
public function test_validate_file( $file, $allowed_files, $expected ) {
$this->assertSame( $expected, validate_file( $file, $allowed_files ) );
}
/**
* Data provider for file validation.
*
* @return array {
* @type array $0... {
* @type string $0 File path.
* @type array $1 List of allowed files.
* @type int $2 Expected result.
* }
* }
*/
public function data_test_validate_file() {
return array(
// Allowed files:
array(
null,
array(),
0,
),
array(
'',
array(),
0,
),
array(
' ',
array(),
0,
),
array(
'.',
array(),
0,
),
array(
'..',
array(),
0,
),
array(
'./',
array(),
0,
),
array(
'foo.ext',
array( 'foo.ext' ),
0,
),
array(
'dir/foo.ext',
array(),
0,
),
array(
'foo..ext',
array(),
0,
),
array(
'dir/dir/../',
array(),
0,
),
// Directory traversal:
array(
'../',
array(),
1,
),
array(
'../../',
array(),
1,
),
array(
'../file.ext',
array(),
1,
),
array(
'../dir/../',
array(),
1,
),
array(
'/dir/dir/../../',
array(),
1,
),
array(
'/dir/dir/../../',
array( '/dir/dir/../../' ),
1,
),
// Windows drives:
array(
'c:',
array(),
2,
),
array(
'C:/WINDOWS/system32',
array( 'C:/WINDOWS/system32' ),
2,
),
// Disallowed files:
array(
'foo.ext',
array( 'bar.ext' ),
3,
),
array(
'foo.ext',
array( '.ext' ),
3,
),
array(
'path/foo.ext',
array( 'foo.ext' ),
3,
),
);
}
}