Skip to content

Commit 71bc495

Browse files
reimicreimic
committed
Test zip slip and symlinks
1 parent 228977b commit 71bc495

File tree

12 files changed

+171
-146
lines changed

12 files changed

+171
-146
lines changed

‎composer.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
"prefer-stable": true,
33
"require": {
44
"ext-json": "*",
5-
"php": ">=7.0"
5+
"php": ">=7.0",
6+
"ext-zip": "*"
67
},
78
"require-dev": {
89
"yoast/phpunit-polyfills": "2.0.0"

‎src/WordPress/Blueprints/Runner/Step/RmStepRunner.php

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,12 @@ class RmStepRunner extends BaseStepRunner {
1212
/**
1313
* @param RmStep $input
1414
*/
15-
public function run( $input ) {
16-
$resolvedPath = $this->getRuntime()->resolvePath( $input->path );
17-
$fileSystem = new Filesystem();
18-
if ( false === $fileSystem->exists( $resolvedPath ) ) {
19-
throw new BlueprintException( "Failed to remove \"$resolvedPath\": the directory or file does not exist." );
20-
}
21-
try {
22-
$fileSystem->remove( $resolvedPath );
23-
} catch ( IOException $exception ) {
24-
throw new BlueprintException( "Failed to remove the directory or file at \"$resolvedPath\"", 0, $exception );
15+
public function run( RmStep $input ) {
16+
$resolved_path = $this->getRuntime()->resolvePath( $input->path );
17+
$filesystem = new Filesystem();
18+
if ( false === $filesystem->exists( $resolved_path ) ) {
19+
throw new BlueprintException( "Failed to remove \"$resolved_path\": the directory or file does not exist." );
2520
}
21+
$filesystem->remove( $resolved_path );
2622
}
2723
}

‎src/WordPress/Blueprints/Runner/Step/UnzipStepRunner.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ class UnzipStepRunner extends BaseStepRunner {
1616
* @return void
1717
*/
1818
public function run(
19-
$input,
20-
$progress_tracker
19+
UnzipStep $input,
20+
Tracker $progress_tracker
2121
) {
2222
$progress_tracker->set( 10, 'Unzipping...' );
2323

‎src/WordPress/Zip/ZipCentralDirectoryEntry.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public function __construct(
5959
$this->versionNeeded = $versionNeeded;
6060
$this->versionCreated = $versionCreated;
6161
$this->firstByteAt = $firstByteAt;
62-
$this->isDirectory = $this->path[- 1] === '/';
62+
$this->isDirectory = substr( $this->path, -1 ) === '/';
6363
}
6464

6565
public function isFileEntry() {

‎src/WordPress/Zip/ZipException.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
3+
namespace WordPress\Zip;
4+
5+
use Exception;
6+
7+
class ZipException extends Exception {}

‎src/WordPress/Zip/ZipFileEntry.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ public function __construct(
7676
$this->compressionMethod = $compressionMethod;
7777
$this->generalPurpose = $generalPurpose;
7878
$this->version = $version;
79-
$this->isDirectory = $this->path[- 1] === '/';
79+
$this->isDirectory = substr( $this->path, -1 ) === '/';
8080
}
8181

8282
public function isFileEntry() {

‎src/WordPress/Zip/functions.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,16 @@ function zip_extract_to( $fp, $to_path ) {
1414
continue;
1515
}
1616

17+
// prevent zip slip -> using relative path to access otherwise inaccessible files
18+
if ( false !== strpos( $entry->path ,'..') ) {
19+
throw new ZipException("Relative paths in zips are not allowed.");
20+
}
21+
22+
// prevent zip with symlinks -> using a symbolic link to access otherwise inaccessible files
23+
if ( is_link( $entry->path ) ) {
24+
throw new ZipException("Semantic links in zips are not allowed.");
25+
}
26+
1727
$path = Path::canonicalize( $to_path . '/' . $entry->path );
1828
$parent = Path::getDirectory( $path );
1929
if ( ! is_dir( $parent ) ) {

‎tests/unit/steps/RmStepRunnerTest.php

Lines changed: 79 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,11 @@
1010
use WordPress\Blueprints\Runner\Step\RmStepRunner;
1111
use WordPress\Blueprints\Runtime\Runtime;
1212

13-
class RmStepRunnerTest extends PHPUnitTestCase
14-
{
13+
class RmStepRunnerTest extends PHPUnitTestCase {
1514
/**
1615
* @var string
1716
*/
18-
private $documentRoot;
17+
private $document_root;
1918

2019
/**
2120
* @var Runtime
@@ -25,151 +24,140 @@ class RmStepRunnerTest extends PHPUnitTestCase
2524
/**
2625
* @var RmStepRunner
2726
*/
28-
private $step;
27+
private $step_runner;
2928

3029
/**
3130
* @var Filesystem
3231
*/
33-
private $fileSystem;
32+
private $filesystem;
3433

3534
/**
3635
* @before
3736
*/
38-
public function before()
39-
{
40-
$this->documentRoot = Path::makeAbsolute("test", sys_get_temp_dir());
41-
$this->runtime = new Runtime($this->documentRoot);
37+
public function before() {
38+
$this->document_root = Path::makeAbsolute( "test", sys_get_temp_dir() );
39+
$this->runtime = new Runtime( $this->document_root );
4240

43-
$this->step = new RmStepRunner();
44-
$this->step->setRuntime($this->runtime);
41+
$this->step_runner = new RmStepRunner();
42+
$this->step_runner->setRuntime( $this->runtime );
4543

46-
$this->fileSystem = new Filesystem();
44+
$this->filesystem = new Filesystem();
4745
}
4846

4947
/**
5048
* @after
5149
*/
52-
public function after()
53-
{
54-
$this->fileSystem->remove($this->documentRoot);
50+
public function after() {
51+
$this->filesystem->remove( $this->document_root );
5552
}
5653

57-
public function testRemoveDirectoryWhenUsingAbsolutePath()
58-
{
59-
$absolutePath = $this->runtime->resolvePath("dir");
60-
$this->fileSystem->mkdir($absolutePath);
54+
public function testRemoveDirectoryWhenUsingAbsolutePath() {
55+
$absolute_path = $this->runtime->resolvePath( "dir" );
56+
$this->filesystem->mkdir( $absolute_path );
6157

62-
$input = new RmStep();
63-
$input->path = $absolutePath;
58+
$step = new RmStep();
59+
$step->path = $absolute_path;
6460

65-
$this->step->run($input);
61+
$this->step_runner->run( $step );
6662

67-
$this->assertDirectoryDoesNotExist($absolutePath);
63+
self::assertDirectoryDoesNotExist( $absolute_path );
6864
}
6965

70-
public function testRemoveDirectoryWhenUsingRelativePath()
71-
{
72-
$relativePath = "dir";
73-
$absolutePath = $this->runtime->resolvePath($relativePath);
74-
$this->fileSystem->mkdir($absolutePath);
66+
public function testRemoveDirectoryWhenUsingRelativePath() {
67+
$relative_path = "dir";
68+
$absolute_path = $this->runtime->resolvePath( $relative_path );
69+
$this->filesystem->mkdir( $absolute_path );
7570

76-
$input = new RmStep();
77-
$input->path = $relativePath;
71+
$step = new RmStep();
72+
$step->path = $relative_path;
7873

79-
$this->step->run($input);
74+
$this->step_runner->run( $step );
8075

81-
$this->assertDirectoryDoesNotExist($absolutePath);
76+
self::assertDirectoryDoesNotExist( $absolute_path );
8277
}
8378

84-
public function testRemoveDirectoryWithSubdirectory()
85-
{
86-
$relativePath = "dir/subdir";
87-
$absolutePath = $this->runtime->resolvePath($relativePath);
88-
$this->fileSystem->mkdir($absolutePath);
79+
public function testRemoveDirectoryWithSubdirectory() {
80+
$relative_path = "dir/subdir";
81+
$absolute_path = $this->runtime->resolvePath( $relative_path );
82+
$this->filesystem->mkdir( $absolute_path );
8983

90-
$input = new RmStep();
91-
$input->path = dirname($relativePath);
84+
$step = new RmStep();
85+
$step->path = dirname( $relative_path );
9286

93-
$this->step->run($input);
87+
$this->step_runner->run( $step );
9488

95-
$this->assertDirectoryDoesNotExist($absolutePath);
89+
self::assertDirectoryDoesNotExist( $absolute_path );
9690
}
9791

98-
public function testRemoveDirectoryWithFile()
99-
{
100-
$relativePath = "dir/file.txt";
101-
$absolutePath = $this->runtime->resolvePath($relativePath);
102-
$this->fileSystem->dumpFile($absolutePath, "test");
92+
public function testRemoveDirectoryWithFile() {
93+
$relative_path = "dir/file.txt";
94+
$absolute_pPath = $this->runtime->resolvePath( $relative_path );
95+
$this->filesystem->dumpFile( $absolute_pPath, "test" );
10396

104-
$input = new RmStep();
105-
$input->path = dirname($relativePath);
97+
$step = new RmStep();
98+
$step->path = dirname( $relative_path );
10699

107-
$this->step->run($input);
100+
$this->step_runner->run( $step );
108101

109-
$this->assertDirectoryDoesNotExist(dirname($absolutePath));
102+
self::assertDirectoryDoesNotExist( dirname( $absolute_pPath ) );
110103
}
111104

112-
public function testRemoveFile()
113-
{
114-
$relativePath = "file.txt";
115-
$absolutePath = $this->runtime->resolvePath($relativePath);
116-
$this->fileSystem->dumpFile($absolutePath, "test");
105+
public function testRemoveFile() {
106+
$relative_path = "file.txt";
107+
$absolute_path = $this->runtime->resolvePath( $relative_path );
108+
$this->filesystem->dumpFile( $absolute_path, "test" );
117109

118-
$input = new RmStep();
119-
$input->path = $relativePath;
110+
$step = new RmStep();
111+
$step->path = $relative_path;
120112

121-
$this->step->run($input);
113+
$this->step_runner->run( $step );
122114

123-
$this->assertDirectoryDoesNotExist($absolutePath);
115+
self::assertDirectoryDoesNotExist( $absolute_path );
124116
}
125117

126-
public function testThrowExceptionWhenRemovingNonexistentDirectoryAndUsingRelativePath()
127-
{
128-
$relativePath = "dir";
129-
$absolutePath = $this->runtime->resolvePath($relativePath);
118+
public function testThrowExceptionWhenRemovingNonexistentDirectoryAndUsingRelativePath() {
119+
$relative_path = "dir";
120+
$absolute_path = $this->runtime->resolvePath( $relative_path );
130121

131-
$input = new RmStep();
132-
$input->path = $relativePath;
122+
$step = new RmStep();
123+
$step->path = $relative_path;
133124

134-
$this->expectException(BlueprintException::class);
135-
$this->expectExceptionMessage("Failed to remove \"$absolutePath\": the directory or file does not exist.");
136-
$this->step->run($input);
125+
self::expectException( BlueprintException::class );
126+
self::expectExceptionMessage( "Failed to remove \"$absolute_path\": the directory or file does not exist." );
127+
$this->step_runner->run( $step );
137128
}
138129

139-
public function testThrowExceptionWhenRemovingNonexistentDirectoryAndUsingAbsolutePath()
140-
{
141-
$absolutePath = "/dir";
130+
public function testThrowExceptionWhenRemovingNonexistentDirectoryAndUsingAbsolutePath() {
131+
$absolute_path = "/dir";
142132

143-
$input = new RmStep();
144-
$input->path = $absolutePath;
133+
$step = new RmStep();
134+
$step->path = $absolute_path;
145135

146-
$this->expectException(BlueprintException::class);
147-
$this->expectExceptionMessage("Failed to remove \"$absolutePath\": the directory or file does not exist.");
148-
$this->step->run($input);
136+
self::expectException( BlueprintException::class );
137+
self::expectExceptionMessage( "Failed to remove \"$absolute_path\": the directory or file does not exist." );
138+
$this->step_runner->run( $step );
149139
}
150140

151-
public function testThrowExceptionWhenRemovingNonexistentFileAndUsingAbsolutePath()
152-
{
153-
$relativePath = "/file.txt";
141+
public function testThrowExceptionWhenRemovingNonexistentFileAndUsingAbsolutePath() {
142+
$relative_path = "/file.txt";
154143

155-
$input = new RmStep();
156-
$input->path = $relativePath;
144+
$step = new RmStep();
145+
$step->path = $relative_path;
157146

158-
$this->expectException(BlueprintException::class);
159-
$this->expectExceptionMessage("Failed to remove \"$relativePath\": the directory or file does not exist.");
160-
$this->step->run($input);
147+
self::expectException( BlueprintException::class );
148+
self::expectExceptionMessage( "Failed to remove \"$relative_path\": the directory or file does not exist." );
149+
$this->step_runner->run( $step );
161150
}
162151

163-
public function testThrowExceptionWhenRemovingNonexistentFileAndUsingRelativePath()
164-
{
152+
public function testThrowExceptionWhenRemovingNonexistentFileAndUsingRelativePath() {
165153
$relativePath = "file.txt";
166154
$absolutePath = $this->runtime->resolvePath($relativePath);
167155

168-
$input = new RmStep();
169-
$input->path = $relativePath;
156+
$step = new RmStep();
157+
$step->path = $relativePath;
170158

171-
$this->expectException(BlueprintException::class);
172-
$this->expectExceptionMessage("Failed to remove \"$absolutePath\": the directory or file does not exist.");
173-
$this->step->run($input);
159+
self::expectException( BlueprintException::class );
160+
self::expectExceptionMessage( "Failed to remove \"$absolutePath\": the directory or file does not exist." );
161+
$this->step_runner->run( $step );
174162
}
175163
}

0 commit comments

Comments
 (0)