Skip to content

Commit b2765da

Browse files
Copilotswissspidy
andauthored
Hardening: Block path traversal in package install via composer.json name (#237)
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Pascal Birchler <pascalb@google.com>
1 parent 46e1423 commit b2765da

2 files changed

Lines changed: 73 additions & 1 deletion

File tree

features/package-install.feature

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1417,6 +1417,29 @@ Feature: Install WP-CLI packages
14171417
"""
14181418
And STDOUT should be empty
14191419
1420+
Scenario: Reject a ZIP package whose composer.json name contains path-traversal components
1421+
Given an empty directory
1422+
And a create-malicious-package-zip.php file:
1423+
"""
1424+
<?php
1425+
$zip = new ZipArchive();
1426+
$zip->open( 'traversal.zip', ZipArchive::CREATE );
1427+
$zip->addFromString( 'traversal/composer.json', '{"name":"..","description":"path traversal test","type":"wp-cli-package"}' );
1428+
$zip->addFromString( 'traversal/vendor/autoload.php', '<?php /* COMPROMISED */ ?>' );
1429+
$zip->close();
1430+
"""
1431+
When I run `php create-malicious-package-zip.php`
1432+
1433+
And I try `wp package install traversal.zip`
1434+
Then the return code should be 1
1435+
And STDERR should contain:
1436+
"""
1437+
Error: Invalid package name '..':
1438+
"""
1439+
And STDOUT should be empty
1440+
And the {PACKAGE_PATH}vendor/autoload.php file should not exist
1441+
And the {PACKAGE_PATH}local directory should not exist
1442+
14201443
@github-api
14211444
Scenario: Install package with --no-interaction fails fast on Git authentication errors
14221445
Given an empty directory

src/Package_Command.php

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use Composer\Installer;
99
use Composer\Json\JsonFile;
1010
use Composer\Package\BasePackage;
11+
use Composer\Package\Loader\ValidatingArrayLoader;
1112
use Composer\Package\PackageInterface;
1213
use Composer\Package\Version\VersionSelector;
1314
use Composer\Repository;
@@ -302,6 +303,10 @@ public function install( $args, $assoc_args ) {
302303
// Move to a location based on the package name
303304
$local_dir = rtrim( WP_CLI::get_runner()->get_packages_dir_path(), '/' ) . '/local/';
304305
$actual_dir_package = $local_dir . str_replace( '/', '-', $package_name );
306+
// Guard against path traversal: ensure destination stays within local_dir.
307+
if ( ! self::is_child_path( $actual_dir_package, $local_dir ) ) {
308+
throw new Exception( 'Invalid package: resolved destination path escapes the packages directory.' );
309+
}
305310
Extractor::copy_overwrite_files( $dir_package, $actual_dir_package );
306311
Extractor::rmdir( $dir_package );
307312
// Behold, the extracted package
@@ -1182,13 +1187,57 @@ private static function get_package_name_and_version_from_dir_package( $dir_pack
11821187
WP_CLI::error( sprintf( "Invalid package: no name in composer.json file '%s'.", $composer_file ) );
11831188
}
11841189
$package_name = $composer_data['name'];
1185-
$version = self::DEFAULT_DEV_BRANCH_CONSTRAINTS;
1190+
$naming_error = ValidatingArrayLoader::hasPackageNamingError( $package_name );
1191+
if ( null !== $naming_error ) {
1192+
WP_CLI::error( sprintf( "Invalid package name '%s': %s", $package_name, $naming_error ) );
1193+
}
1194+
$version = self::DEFAULT_DEV_BRANCH_CONSTRAINTS;
11861195
if ( ! empty( $composer_data['version'] ) ) {
11871196
$version = $composer_data['version'];
11881197
}
11891198
return [ $package_name, $version ];
11901199
}
11911200

1201+
/**
1202+
* Checks whether a path is inside (or equal to) a given parent directory.
1203+
*
1204+
* Resolves '.' and '..' segments without touching the filesystem so it
1205+
* works even when the paths do not exist yet. Uses a case-insensitive
1206+
* comparison on Windows where the filesystem is case-insensitive.
1207+
*
1208+
* @param string $path Path to test.
1209+
* @param string $parent_dir Parent directory to test against.
1210+
* @return bool True when $path is inside $parent_dir.
1211+
*/
1212+
private static function is_child_path( $path, $parent_dir ) {
1213+
$normalized_path = self::resolve_dot_segments( rtrim( str_replace( '\\', '/', $path ), '/' ) ) . '/';
1214+
$normalized_parent = self::resolve_dot_segments( rtrim( str_replace( '\\', '/', $parent_dir ), '/' ) ) . '/';
1215+
if ( DIRECTORY_SEPARATOR === '\\' ) {
1216+
return 0 === stripos( $normalized_path, $normalized_parent );
1217+
}
1218+
return 0 === strpos( $normalized_path, $normalized_parent );
1219+
}
1220+
1221+
/**
1222+
* Resolves '.' and '..' segments in a path without touching the filesystem.
1223+
*
1224+
* @param string $path Forward-slash path to resolve.
1225+
* @return string Resolved path.
1226+
*/
1227+
private static function resolve_dot_segments( $path ) {
1228+
$is_absolute = isset( $path[0] ) && '/' === $path[0];
1229+
$result = [];
1230+
foreach ( explode( '/', $path ) as $part ) {
1231+
if ( '..' === $part ) {
1232+
array_pop( $result );
1233+
} elseif ( '.' !== $part && '' !== $part ) {
1234+
$result[] = $part;
1235+
}
1236+
}
1237+
$resolved = implode( '/', $result );
1238+
return $is_absolute ? '/' . $resolved : $resolved;
1239+
}
1240+
11921241
/**
11931242
* Gets the WP-CLI packages composer.json object.
11941243
*/

0 commit comments

Comments
 (0)