diff --git a/features/package-install.feature b/features/package-install.feature index ff76ccbe..262e93d1 100644 --- a/features/package-install.feature +++ b/features/package-install.feature @@ -1417,6 +1417,29 @@ Feature: Install WP-CLI packages """ And STDOUT should be empty + Scenario: Reject a ZIP package whose composer.json name contains path-traversal components + Given an empty directory + And a create-malicious-package-zip.php file: + """ + open( 'traversal.zip', ZipArchive::CREATE ); + $zip->addFromString( 'traversal/composer.json', '{"name":"..","description":"path traversal test","type":"wp-cli-package"}' ); + $zip->addFromString( 'traversal/vendor/autoload.php', '' ); + $zip->close(); + """ + When I run `php create-malicious-package-zip.php` + + And I try `wp package install traversal.zip` + Then the return code should be 1 + And STDERR should contain: + """ + Error: Invalid package name '..': + """ + And STDOUT should be empty + And the {PACKAGE_PATH}vendor/autoload.php file should not exist + And the {PACKAGE_PATH}local directory should not exist + @github-api Scenario: Install package with --no-interaction fails fast on Git authentication errors Given an empty directory diff --git a/src/Package_Command.php b/src/Package_Command.php index 1de39a8b..7e779643 100644 --- a/src/Package_Command.php +++ b/src/Package_Command.php @@ -8,6 +8,7 @@ use Composer\Installer; use Composer\Json\JsonFile; use Composer\Package\BasePackage; +use Composer\Package\Loader\ValidatingArrayLoader; use Composer\Package\PackageInterface; use Composer\Package\Version\VersionSelector; use Composer\Repository; @@ -302,6 +303,10 @@ public function install( $args, $assoc_args ) { // Move to a location based on the package name $local_dir = rtrim( WP_CLI::get_runner()->get_packages_dir_path(), '/' ) . '/local/'; $actual_dir_package = $local_dir . str_replace( '/', '-', $package_name ); + // Guard against path traversal: ensure destination stays within local_dir. + if ( ! self::is_child_path( $actual_dir_package, $local_dir ) ) { + throw new Exception( 'Invalid package: resolved destination path escapes the packages directory.' ); + } Extractor::copy_overwrite_files( $dir_package, $actual_dir_package ); Extractor::rmdir( $dir_package ); // Behold, the extracted package @@ -1182,13 +1187,57 @@ private static function get_package_name_and_version_from_dir_package( $dir_pack WP_CLI::error( sprintf( "Invalid package: no name in composer.json file '%s'.", $composer_file ) ); } $package_name = $composer_data['name']; - $version = self::DEFAULT_DEV_BRANCH_CONSTRAINTS; + $naming_error = ValidatingArrayLoader::hasPackageNamingError( $package_name ); + if ( null !== $naming_error ) { + WP_CLI::error( sprintf( "Invalid package name '%s': %s", $package_name, $naming_error ) ); + } + $version = self::DEFAULT_DEV_BRANCH_CONSTRAINTS; if ( ! empty( $composer_data['version'] ) ) { $version = $composer_data['version']; } return [ $package_name, $version ]; } + /** + * Checks whether a path is inside (or equal to) a given parent directory. + * + * Resolves '.' and '..' segments without touching the filesystem so it + * works even when the paths do not exist yet. Uses a case-insensitive + * comparison on Windows where the filesystem is case-insensitive. + * + * @param string $path Path to test. + * @param string $parent_dir Parent directory to test against. + * @return bool True when $path is inside $parent_dir. + */ + private static function is_child_path( $path, $parent_dir ) { + $normalized_path = self::resolve_dot_segments( rtrim( str_replace( '\\', '/', $path ), '/' ) ) . '/'; + $normalized_parent = self::resolve_dot_segments( rtrim( str_replace( '\\', '/', $parent_dir ), '/' ) ) . '/'; + if ( DIRECTORY_SEPARATOR === '\\' ) { + return 0 === stripos( $normalized_path, $normalized_parent ); + } + return 0 === strpos( $normalized_path, $normalized_parent ); + } + + /** + * Resolves '.' and '..' segments in a path without touching the filesystem. + * + * @param string $path Forward-slash path to resolve. + * @return string Resolved path. + */ + private static function resolve_dot_segments( $path ) { + $is_absolute = isset( $path[0] ) && '/' === $path[0]; + $result = []; + foreach ( explode( '/', $path ) as $part ) { + if ( '..' === $part ) { + array_pop( $result ); + } elseif ( '.' !== $part && '' !== $part ) { + $result[] = $part; + } + } + $resolved = implode( '/', $result ); + return $is_absolute ? '/' . $resolved : $resolved; + } + /** * Gets the WP-CLI packages composer.json object. */