Test zip slip and symlinks

WordPress · Mar 28, 2024 · 71bc495 · 71bc495
1 parent 228977b
commit 71bc495
Show file tree

Hide file tree

Showing 12 changed files with 171 additions and 146 deletions.
diff --git a/composer.json b/composer.json
@@ -2,7 +2,8 @@
   "prefer-stable": true,
   "require": {
     "ext-json": "*",
-    "php": ">=7.0"
+    "php": ">=7.0",
+    "ext-zip": "*"
   },
   "require-dev": {
     "yoast/phpunit-polyfills": "2.0.0"

diff --git a/src/WordPress/Blueprints/Runner/Step/RmStepRunner.php b/src/WordPress/Blueprints/Runner/Step/RmStepRunner.php
@@ -12,16 +12,12 @@ class RmStepRunner extends BaseStepRunner {
 	/**
 	 * @param RmStep $input
 	 */
-	public function run( $input ) {
-		$resolvedPath = $this->getRuntime()->resolvePath( $input->path );
-		$fileSystem   = new Filesystem();
-		if ( false === $fileSystem->exists( $resolvedPath ) ) {
-			throw new BlueprintException( "Failed to remove \"$resolvedPath\": the directory or file does not exist." );
-		}
-		try {
-			$fileSystem->remove( $resolvedPath );
-		} catch ( IOException $exception ) {
-			throw new BlueprintException( "Failed to remove the directory or file at \"$resolvedPath\"", 0, $exception );
+	public function run( RmStep $input ) {
+		$resolved_path = $this->getRuntime()->resolvePath( $input->path );
+		$filesystem   = new Filesystem();
+		if ( false === $filesystem->exists( $resolved_path ) ) {
+			throw new BlueprintException( "Failed to remove \"$resolved_path\": the directory or file does not exist." );
 		}
+		$filesystem->remove( $resolved_path );
 	}
 }
diff --git a/src/WordPress/Blueprints/Runner/Step/UnzipStepRunner.php b/src/WordPress/Blueprints/Runner/Step/UnzipStepRunner.php
@@ -16,8 +16,8 @@ class UnzipStepRunner extends BaseStepRunner {
 	 * @return void
 	 */
 	public function run(
-		$input,
-		$progress_tracker
+		UnzipStep $input,
+		Tracker   $progress_tracker
 	) {
 		$progress_tracker->set( 10, 'Unzipping...' );
 

diff --git a/src/WordPress/Zip/ZipCentralDirectoryEntry.php b/src/WordPress/Zip/ZipCentralDirectoryEntry.php
@@ -59,7 +59,7 @@ public function __construct(
 		$this->versionNeeded      = $versionNeeded;
 		$this->versionCreated     = $versionCreated;
 		$this->firstByteAt        = $firstByteAt;
-		$this->isDirectory        = $this->path[- 1] === '/';
+		$this->isDirectory        = substr( $this->path, -1 ) === '/';
 	}
 
 	public function isFileEntry() {

diff --git a/src/WordPress/Zip/ZipException.php b/src/WordPress/Zip/ZipException.php
@@ -0,0 +1,7 @@
+<?php
+
+namespace WordPress\Zip;
+
+use Exception;
+
+class ZipException extends Exception {}
diff --git a/src/WordPress/Zip/ZipFileEntry.php b/src/WordPress/Zip/ZipFileEntry.php
@@ -76,7 +76,7 @@ public function __construct(
 		$this->compressionMethod = $compressionMethod;
 		$this->generalPurpose    = $generalPurpose;
 		$this->version           = $version;
-		$this->isDirectory       = $this->path[- 1] === '/';
+		$this->isDirectory       = substr( $this->path, -1 ) === '/';
 	}
 
 	public function isFileEntry() {

diff --git a/src/WordPress/Zip/functions.php b/src/WordPress/Zip/functions.php
@@ -14,6 +14,16 @@ function zip_extract_to( $fp, $to_path ) {
 			continue;
 		}
 
+		// prevent zip slip -> using relative path to access otherwise inaccessible files
+		if ( false !== strpos( $entry->path ,'..') ) {
+			throw new ZipException("Relative paths in zips are not allowed.");
+		}
+
+		// prevent zip with symlinks -> using a symbolic link to access otherwise inaccessible files
+		if ( is_link( $entry->path ) ) {
+			throw new ZipException("Semantic links in zips are not allowed.");
+		}
+
 		$path   = Path::canonicalize( $to_path . '/' . $entry->path );
 		$parent = Path::getDirectory( $path );
 		if ( ! is_dir( $parent ) ) {

diff --git a/tests/unit/steps/RmStepRunnerTest.php b/tests/unit/steps/RmStepRunnerTest.php
@@ -10,12 +10,11 @@
 use WordPress\Blueprints\Runner\Step\RmStepRunner;
 use WordPress\Blueprints\Runtime\Runtime;
 
-class RmStepRunnerTest extends PHPUnitTestCase
-{
+class RmStepRunnerTest extends PHPUnitTestCase {
     /**
      * @var string
      */
-    private $documentRoot;
+    private $document_root;
 
     /**
      * @var Runtime
@@ -25,151 +24,140 @@ class RmStepRunnerTest extends PHPUnitTestCase
     /**
      * @var RmStepRunner
      */
-    private $step;
+    private $step_runner;
 
     /**
      * @var Filesystem
      */
-    private $fileSystem;
+    private $filesystem;
 
     /**
      * @before
      */
-    public function before()
-    {
-        $this->documentRoot = Path::makeAbsolute("test", sys_get_temp_dir());
-        $this->runtime = new Runtime($this->documentRoot);
+    public function before() {
+        $this->document_root = Path::makeAbsolute( "test", sys_get_temp_dir() );
+        $this->runtime = new Runtime( $this->document_root );
 
-        $this->step = new RmStepRunner();
-        $this->step->setRuntime($this->runtime);
+        $this->step_runner = new RmStepRunner();
+        $this->step_runner->setRuntime( $this->runtime );
 
-        $this->fileSystem = new Filesystem();
+        $this->filesystem = new Filesystem();
     }
 
     /**
      * @after
      */
-    public function after()
-    {
-        $this->fileSystem->remove($this->documentRoot);
+    public function after() {
+        $this->filesystem->remove( $this->document_root );
     }
 
-    public function testRemoveDirectoryWhenUsingAbsolutePath()
-    {
-        $absolutePath = $this->runtime->resolvePath("dir");
-        $this->fileSystem->mkdir($absolutePath);
+    public function testRemoveDirectoryWhenUsingAbsolutePath() {
+        $absolute_path = $this->runtime->resolvePath( "dir" );
+        $this->filesystem->mkdir( $absolute_path );
 
-        $input = new RmStep();
-        $input->path = $absolutePath;
+        $step = new RmStep();
+        $step->path = $absolute_path;
 
-        $this->step->run($input);
+        $this->step_runner->run( $step );
 
-        $this->assertDirectoryDoesNotExist($absolutePath);
+		self::assertDirectoryDoesNotExist( $absolute_path );
     }
 
-    public function testRemoveDirectoryWhenUsingRelativePath()
-    {
-        $relativePath = "dir";
-        $absolutePath = $this->runtime->resolvePath($relativePath);
-        $this->fileSystem->mkdir($absolutePath);
+    public function testRemoveDirectoryWhenUsingRelativePath() {
+        $relative_path = "dir";
+        $absolute_path = $this->runtime->resolvePath( $relative_path );
+        $this->filesystem->mkdir( $absolute_path );
 
-        $input = new RmStep();
-        $input->path = $relativePath;
+        $step = new RmStep();
+        $step->path = $relative_path;
 
-        $this->step->run($input);
+        $this->step_runner->run( $step );
 
-        $this->assertDirectoryDoesNotExist($absolutePath);
+        self::assertDirectoryDoesNotExist( $absolute_path );
     }
 
-    public function testRemoveDirectoryWithSubdirectory()
-    {
-        $relativePath = "dir/subdir";
-        $absolutePath = $this->runtime->resolvePath($relativePath);
-        $this->fileSystem->mkdir($absolutePath);
+    public function testRemoveDirectoryWithSubdirectory() {
+        $relative_path = "dir/subdir";
+        $absolute_path = $this->runtime->resolvePath( $relative_path );
+        $this->filesystem->mkdir( $absolute_path );
 
-        $input = new RmStep();
-        $input->path = dirname($relativePath);
+        $step = new RmStep();
+        $step->path = dirname( $relative_path );
 
-        $this->step->run($input);
+        $this->step_runner->run( $step );
 
-        $this->assertDirectoryDoesNotExist($absolutePath);
+        self::assertDirectoryDoesNotExist( $absolute_path );
     }
 
-    public function testRemoveDirectoryWithFile()
-    {
-        $relativePath = "dir/file.txt";
-        $absolutePath = $this->runtime->resolvePath($relativePath);
-        $this->fileSystem->dumpFile($absolutePath, "test");
+    public function testRemoveDirectoryWithFile() {
+        $relative_path = "dir/file.txt";
+        $absolute_pPath = $this->runtime->resolvePath( $relative_path );
+        $this->filesystem->dumpFile( $absolute_pPath, "test" );
 
-        $input = new RmStep();
-        $input->path = dirname($relativePath);
+        $step = new RmStep();
+        $step->path = dirname( $relative_path );
 
-        $this->step->run($input);
+        $this->step_runner->run( $step );
 
-        $this->assertDirectoryDoesNotExist(dirname($absolutePath));
+        self::assertDirectoryDoesNotExist( dirname( $absolute_pPath ) );
     }
 
-    public function testRemoveFile()
-    {
-        $relativePath = "file.txt";
-        $absolutePath = $this->runtime->resolvePath($relativePath);
-        $this->fileSystem->dumpFile($absolutePath, "test");
+    public function testRemoveFile() {
+        $relative_path = "file.txt";
+        $absolute_path = $this->runtime->resolvePath( $relative_path );
+        $this->filesystem->dumpFile( $absolute_path, "test" );
 
-        $input = new RmStep();
-        $input->path = $relativePath;
+        $step = new RmStep();
+        $step->path = $relative_path;
 
-        $this->step->run($input);
+        $this->step_runner->run( $step );
 
-        $this->assertDirectoryDoesNotExist($absolutePath);
+        self::assertDirectoryDoesNotExist( $absolute_path );
     }
 
-    public function testThrowExceptionWhenRemovingNonexistentDirectoryAndUsingRelativePath()
-    {
-        $relativePath = "dir";
-        $absolutePath = $this->runtime->resolvePath($relativePath);
+    public function testThrowExceptionWhenRemovingNonexistentDirectoryAndUsingRelativePath() {
+        $relative_path = "dir";
+        $absolute_path = $this->runtime->resolvePath( $relative_path );
 
-        $input = new RmStep();
-        $input->path = $relativePath;
+        $step = new RmStep();
+        $step->path = $relative_path;
 
-        $this->expectException(BlueprintException::class);
-        $this->expectExceptionMessage("Failed to remove \"$absolutePath\": the directory or file does not exist.");
-        $this->step->run($input);
+        self::expectException( BlueprintException::class );
+        self::expectExceptionMessage( "Failed to remove \"$absolute_path\": the directory or file does not exist." );
+        $this->step_runner->run( $step );
     }
 
-    public function testThrowExceptionWhenRemovingNonexistentDirectoryAndUsingAbsolutePath()
-    {
-        $absolutePath = "/dir";
+    public function testThrowExceptionWhenRemovingNonexistentDirectoryAndUsingAbsolutePath() {
+        $absolute_path = "/dir";
 
-        $input = new RmStep();
-        $input->path = $absolutePath;
+        $step = new RmStep();
+        $step->path = $absolute_path;
 
-        $this->expectException(BlueprintException::class);
-        $this->expectExceptionMessage("Failed to remove \"$absolutePath\": the directory or file does not exist.");
-        $this->step->run($input);
+		self::expectException( BlueprintException::class );
+		self::expectExceptionMessage( "Failed to remove \"$absolute_path\": the directory or file does not exist." );
+        $this->step_runner->run( $step );
     }
 
-    public function testThrowExceptionWhenRemovingNonexistentFileAndUsingAbsolutePath()
-    {
-        $relativePath = "/file.txt";
+    public function testThrowExceptionWhenRemovingNonexistentFileAndUsingAbsolutePath() {
+        $relative_path = "/file.txt";
 
-        $input = new RmStep();
-        $input->path = $relativePath;
+        $step = new RmStep();
+        $step->path = $relative_path;
 
-        $this->expectException(BlueprintException::class);
-        $this->expectExceptionMessage("Failed to remove \"$relativePath\": the directory or file does not exist.");
-        $this->step->run($input);
+		self::expectException( BlueprintException::class );
+		self::expectExceptionMessage( "Failed to remove \"$relative_path\": the directory or file does not exist." );
+        $this->step_runner->run( $step );
     }
 
-    public function testThrowExceptionWhenRemovingNonexistentFileAndUsingRelativePath()
-    {
+    public function testThrowExceptionWhenRemovingNonexistentFileAndUsingRelativePath() {
         $relativePath = "file.txt";
         $absolutePath = $this->runtime->resolvePath($relativePath);
 
-        $input = new RmStep();
-        $input->path = $relativePath;
+        $step = new RmStep();
+        $step->path = $relativePath;
 
-        $this->expectException(BlueprintException::class);
-        $this->expectExceptionMessage("Failed to remove \"$absolutePath\": the directory or file does not exist.");
-        $this->step->run($input);
+		self::expectException( BlueprintException::class );
+		self::expectExceptionMessage( "Failed to remove \"$absolutePath\": the directory or file does not exist." );
+        $this->step_runner->run( $step );
     }
 }