From 62bbca247977bb486f0fe2d7e6c7563167fdd680 Mon Sep 17 00:00:00 2001 From: Paul Bearne Date: Fri, 11 Nov 2022 18:08:28 -0500 Subject: [PATCH 1/2] Added test for wp_mkdir_p --- tests/phpunit/tests/functions/wpMkdirP.php | 51 ++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 tests/phpunit/tests/functions/wpMkdirP.php diff --git a/tests/phpunit/tests/functions/wpMkdirP.php b/tests/phpunit/tests/functions/wpMkdirP.php new file mode 100644 index 0000000000000..29bb56b1df906 --- /dev/null +++ b/tests/phpunit/tests/functions/wpMkdirP.php @@ -0,0 +1,51 @@ +assertTrue( wp_mkdir_p( $target ) ); + $this->assertDirectoryExists( $target ); + } + + public function test_if_file_with_name_extists() { + + $this->assertFalse( wp_mkdir_p( ABSPATH . 'wp-admin/index.php' ) ); + } + + public function test_if_folder_with_name_exists() { + + $this->assertTrue( wp_mkdir_p( ABSPATH . 'wp-admin' ) ); + } + + // should return false ../ in path + // Do not allow path traversals. + public function test_if_up_tree_in_target() { + + $this->assertFalse( wp_mkdir_p( '../wp-admin' ) ); + $this->assertFalse( wp_mkdir_p( '../../test' ) ); + $this->assertFalse( wp_mkdir_p( '..' . DIRECTORY_SEPARATOR . 'test' ) ); + $this->assertFalse( wp_mkdir_p( ABSPATH . 'test/../../' ) ); + + // this resolves to current dir so is found + $this->assertTrue( wp_mkdir_p( '../../../../' ) ); + } + + public function test_permissions_are_set() { + + $upload_dir = wp_upload_dir(); + $target = $upload_dir['basedir'] . 'permission_test'; + $parent_stat = stat( $upload_dir['basedir'] ); + $this->assertTrue( wp_mkdir_p( $target ) ); + $target_stat = stat( $target ); + $this->assertSame( $parent_stat['mode'], $target_stat['mode'] ); + } +} From d87067c54301fc145e95f3bf7a4fde44caf1bb03 Mon Sep 17 00:00:00 2001 From: Paul Bearne Date: Fri, 11 Nov 2022 18:09:57 -0500 Subject: [PATCH 2/2] Upodated code to hard and use str_contains --- src/wp-includes/functions.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/wp-includes/functions.php b/src/wp-includes/functions.php index a38c78ccacae0..ffac68901f774 100644 --- a/src/wp-includes/functions.php +++ b/src/wp-includes/functions.php @@ -2036,7 +2036,7 @@ function wp_mkdir_p( $target ) { } // Do not allow path traversals. - if ( false !== strpos( $target, '../' ) || false !== strpos( $target, '..' . DIRECTORY_SEPARATOR ) ) { + if ( str_contains( $target, '../' ) || str_contains( $target, '..' . DIRECTORY_SEPARATOR ) ) { return false; } @@ -2054,13 +2054,13 @@ function wp_mkdir_p( $target ) { $dir_perms = 0777; } - if ( @mkdir( $target, $dir_perms, true ) ) { + if ( mkdir( $target, $dir_perms, true ) || is_dir( $target ) ) { /* * If a umask is set that modifies $dir_perms, we'll have to re-set * the $dir_perms correctly with chmod() */ - if ( ( $dir_perms & ~umask() ) != $dir_perms ) { + if ( ( $dir_perms & ~umask() ) !== $dir_perms ) { $folder_parts = explode( '/', substr( $target, strlen( $target_parent ) + 1 ) ); for ( $i = 1, $c = count( $folder_parts ); $i <= $c; $i++ ) { chmod( $target_parent . '/' . implode( '/', array_slice( $folder_parts, 0, $i ) ), $dir_perms );