diff --git a/src/wp-includes/functions.php b/src/wp-includes/functions.php index a60bb57df7178..4e06297fac438 100644 --- a/src/wp-includes/functions.php +++ b/src/wp-includes/functions.php @@ -7905,6 +7905,16 @@ function wp_delete_file_from_directory( $file, $directory ) { if ( false !== $real_file ) { $real_file = wp_normalize_path( $real_file ); + + /* + * realpath() resolves `..` segments for real filesystem paths, but it does + * not support stream wrappers, so the stream branch above leaves them in + * place. A surviving `..` segment lets the prefix check below pass while the + * wrapper walks back out of $directory on delete, so reject it here. + */ + if ( preg_match( '#(?:^|/)\.\.(?:/|$)#', $real_file ) ) { + return false; + } } if ( false !== $real_directory ) { diff --git a/tests/phpunit/tests/functions/wpDeleteFileFromDirectory.php b/tests/phpunit/tests/functions/wpDeleteFileFromDirectory.php new file mode 100644 index 0000000000000..33050535602fd --- /dev/null +++ b/tests/phpunit/tests/functions/wpDeleteFileFromDirectory.php @@ -0,0 +1,98 @@ +assertTrue( wp_delete_file_from_directory( $file, $directory ) ); + $this->assertSame( array( $file ), self::$unlinked ); + } + + /** + * A `..` segment must not let a stream-wrapped path escape the directory. + * + * realpath() resolves `..` for real filesystem paths, but is skipped for + * stream wrappers, so the containment check has to reject the traversal + * itself rather than delete a file outside the directory. + */ + public function test_rejects_stream_path_traversal() { + $directory = 'wpdeletetest://bucket/uploads'; + $file = 'wpdeletetest://bucket/uploads/../../secret/keys.json'; + + $this->assertFalse( wp_delete_file_from_directory( $file, $directory ) ); + $this->assertSame( array(), self::$unlinked ); + } + + /** + * A trailing `..` segment is also rejected. + */ + public function test_rejects_trailing_stream_path_traversal() { + $directory = 'wpdeletetest://bucket/uploads'; + $file = 'wpdeletetest://bucket/uploads/subdir/..'; + + $this->assertFalse( wp_delete_file_from_directory( $file, $directory ) ); + $this->assertSame( array(), self::$unlinked ); + } + + /** + * Dots inside a filename are not treated as a traversal. + */ + public function test_allows_dots_within_stream_filename() { + $directory = 'wpdeletetest://bucket/uploads'; + $file = 'wpdeletetest://bucket/uploads/my..archive.zip'; + + $this->assertTrue( wp_delete_file_from_directory( $file, $directory ) ); + $this->assertSame( array( $file ), self::$unlinked ); + } +} + +/** + * Minimal stream wrapper that records the paths passed to unlink(). + */ +class WpDeleteFileFromDirectory_Stream { + + public $context; + + public function unlink( $path ) { + Tests_Functions_WpDeleteFileFromDirectory::$unlinked[] = $path; + return true; + } + + public function url_stat( $path, $flags ) { + return array(); + } + + public function stream_open( $path, $mode, $options, &$opened_path ) { + return true; + } +}