Ticket #42016: 42016.diff

src/wp-admin/includes/file.php

@@ -663 +663 @@
  * @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 )
@@ -1119 +1119 @@
                 if ( '__MACOSX/' === substr($info['name'], 0, 9) ) // Skip the OS X-created __MACOSX directory
                         continue;
 
+                // Don't extract invalid files:
                 if ( 0 !== validate_file( $info['name'] ) ) {
-                        return new WP_Error( 'invalid_file_ziparchive', __( 'Could not extract file from archive.' ), $info['name'] );
+                        continue;
                 }
 
                 $uncompressed_size += $info['size'];
@@ -1180 +1181 @@
                 if ( '__MACOSX/' === substr($info['name'], 0, 9) ) // Don't extract the OS X-created __MACOSX directory files
                         continue;
 
+                // Don't extract invalid files:
+                if ( 0 !== validate_file( $info['name'] ) ) {
+                        continue;
+                }
+
                 $contents = $z->getFromIndex($i);
                 if ( false === $contents )
                         return new WP_Error( 'extract_failed_ziparchive', __( 'Could not extract file from archive.' ), $info['name'] );
@@ -1283 +1289 @@
                 if ( '__MACOSX/' === substr($file['filename'], 0, 9) ) // Don't extract the OS X-created __MACOSX directory files
                         continue;
 
+                // Don't extract invalid files:
                 if ( 0 !== validate_file( $file['filename'] ) ) {
-                        return new WP_Error( 'invalid_file_pclzip', __( 'Could not extract file from archive.' ), $file['filename'] );
+                        continue;
                 }
 
                 if ( ! $wp_filesystem->put_contents( $to . $file['filename'], $file['content'], FS_CHMOD_FILE) )

src/wp-includes/functions.php

@@ -4252 +4252 @@
  * @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;
 

tests/phpunit/tests/functions.php

@@ -1161 +1161 @@
                 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,
+                        ),
+
+                );
+        }
 }