Fix zip slip vulnerability

Author: reimic

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"

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...' );
 

src/WordPress/Zip/functions.php

@@ -14,6 +14,10 @@ function zip_extract_to( $fp, $to_path ) {
 			continue;
 		}
 
+		if ( false !== strpos( $entry->path ,'..') ) {
+			continue;
+		}
+
 		$path   = Path::canonicalize( $to_path . '/' . $entry->path );
 		$parent = Path::getDirectory( $path );
 		if ( ! is_dir( $parent ) ) {

tests/unit/steps/UnzipStepRunnerTest.php

@@ -25,17 +25,17 @@ class UnzipStepRunnerTest extends PHPUnitTestCase {
 	private $runtime;
 
 	/**
-	 * @var UnzipStepRunner $step
+	 * @var UnzipStepRunner $step_runner
 	 */
-	private $step;
+	private $step_runner;
 
 	/**
 	 * @var Filesystem
 	 */
-	private $file_system;
+	private $filesystem;
 
 	/**
-	 * @var Stub
+	 * @var ResourceManager $resource_manager resource manager mock
 	 */
 	private $resource_manager;
 
@@ -48,51 +48,47 @@ public function before() {
 
 		$this->resource_manager = $this->createMock( ResourceManager::class );
 
-		$this->step = new UnzipStepRunner();
-		$this->step->setRuntime( $this->runtime );
-		$this->step->setResourceManager( $this->resource_manager );
+		$this->step_runner = new UnzipStepRunner();
+		$this->step_runner->setRuntime( $this->runtime );
+		$this->step_runner->setResourceManager( $this->resource_manager );
 
-		$this->file_system = new Filesystem();
+		$this->filesystem = new Filesystem();
 	}
 
 	/**
 	 * @after
 	 */
 	public function after() {
-		$this->file_system->remove( $this->document_root );
+		$this->filesystem->remove( $this->document_root );
 	}
 
-	public function test(){
-		$this->assertTrue(true); // Placeholder until true test are fixed.
+	public function testUnzipFileWhenUsingAbsolutePath() {
+		$zip = __DIR__ . '/resources/test_zip.zip';
+		$this->resource_manager->method( 'getStream' )
+			->willReturn( fopen( $zip, 'rb' ) );
+
+		$step = new UnzipStep();
+		$step->setZipFile( $zip );
+		$extracted_file_path = $this->runtime->resolvePath( 'dir/test_zip.txt' );
+		$step->setExtractToPath( Path::getDirectory( $extracted_file_path ) );
+
+		$this->step_runner->run( $step, new Tracker() );
+
+		self::assertFileEquals( __DIR__ . '/resources/test_zip.txt', $extracted_file_path );
 	}
 
-//	public function testUnzipFileWhenUsingAbsolutePath() {
-//		$zip = __DIR__ . '/resources/test_zip.zip';
-//		$this->resource_manager->method( 'getStream' )
-//			->willReturn( fopen( $zip, 'rb' ) );
-//
-//		$input = new UnzipStep();
-//		$input->setZipFile( $zip );
-//		$extracted_file_path = $this->runtime->resolvePath( 'dir/test_zip.txt' );
-//		$input->setExtractToPath( Path::getDirectory( $extracted_file_path ) );
-//
-//		$this->step->run( $input, new Tracker() );
-//
-//		$this->assertFileEquals( __DIR__ . '/resources/test_zip.txt', $extracted_file_path );
-//	}
-//
-//	public function testUnzipFileWhenUsingRelativePath() {
-//		$zip = __DIR__ . '/resources/test_zip.zip';
-//		$this->resource_manager->method( 'getStream' )
-//			->willReturn( fopen( $zip, 'rb' ) );
-//
-//		$input = new UnzipStep();
-//		$input->setZipFile( $zip );
-//		$input->setExtractToPath( 'dir' );
-//
-//		$this->step->run( $input, new Tracker() );
-//
-//		$extracted_file_path = $this->runtime->resolvePath( 'dir/test_zip.txt' );
-//		$this->assertFileEquals( __DIR__ . '/resources/test_zip.txt', $extracted_file_path );
-//	}
+	public function testUnzipFileWhenUsingRelativePath() {
+		$zip = __DIR__ . '/resources/test_zip.zip';
+		$this->resource_manager->method( 'getStream' )
+			->willReturn( fopen( $zip, 'rb' ) );
+
+		$input = new UnzipStep();
+		$input->setZipFile( $zip );
+		$input->setExtractToPath( 'dir' );
+
+		$this->step_runner->run( $input, new Tracker() );
+
+		$extracted_file_path = $this->runtime->resolvePath( 'dir/test_zip.txt' );
+		self::assertFileEquals( __DIR__ . '/resources/test_zip.txt', $extracted_file_path );
+	}
 }

tests/unit/zip/ZipFunctionsTest.php

@@ -0,0 +1,30 @@
+<?php
+
+namespace unit\zip;
+
+use PHPUnitTestCase;
+use Symfony\Component\Filesystem\Filesystem;
+use Symfony\Component\Filesystem\Path;
+use ZipArchive;
+use function WordPress\Zip\zip_extract_to;
+
+class ZipFunctionsTest extends PHPUnitTestCase {
+	public function testImmuneToZipSlip() {
+		$filesystem = new Filesystem();
+
+		$filename = __DIR__ . 'tmp/zip-slip-test.zip';
+		$filesystem->mkdir( dirname( $filename ) );
+
+		$zip = new ZipArchive();
+		$zip->open( $filename, ZipArchive::CREATE );
+		$zip->addFromString( "../../../../../../../../tmp/zip-slip-test.txt" . time(), "zip slip test" );
+		$zip->close();
+
+		zip_extract_to( fopen( $filename, 'rb' ), dirname( $filename ) );
+
+		$slipped_dir = Path::canonicalize(__DIR__ . "../../../../../../../../tmp");
+		self::assertDirectoryDoesNotExist( $slipped_dir );
+
+		$filesystem->remove( dirname( $filename ) );
+	}
+}