Skip to content

Commit 81a39cc

Browse files
authored
fix(Storage): Implement path containment to prevent traversal attacks (#8658)
1 parent 19b2982 commit 81a39cc

3 files changed

Lines changed: 69 additions & 2 deletions

File tree

Storage/src/StorageObject.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -688,9 +688,24 @@ public function downloadAsString(array $options = [])
688688
* If provided one must also include an `encryptionKey`.
689689
* }
690690
* @return StreamInterface
691+
* @throws \RuntimeException
691692
*/
692693
public function downloadToFile($path, array $options = [])
693694
{
695+
// throws an exception in the case of `..` segments, paths
696+
// starting with `/`, and Windows drive letters (e.g., `C:`).
697+
$normalizedPath = str_replace('\\', '/', $path);
698+
$pathSegments = explode('/', $normalizedPath);
699+
700+
if (in_array('..', $pathSegments, true) ||
701+
strpos($normalizedPath, '/') === 0 ||
702+
preg_match('/^[a-zA-Z]:\//', $normalizedPath)
703+
) {
704+
throw new \RuntimeException(
705+
'Path traversal is not allowed. File path is outside the designated directory.'
706+
);
707+
}
708+
694709
$source = $this->downloadAsStream($options);
695710
$destination = Utils::streamFor(fopen($path, 'w'));
696711

Storage/tests/System/ManageObjectsTest.php

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -443,7 +443,7 @@ public function testDownloadsToFileShouldNotCreateFileWhenObjectNotFound()
443443
$objectName = uniqid(self::TESTING_PREFIX);
444444
$testObject = self::$bucket->object($objectName);
445445
$exceptionString = 'No such object';
446-
$downloadFilePath = __DIR__ . '/' . $objectName;
446+
$downloadFilePath = $objectName;
447447

448448
$throws = false;
449449
try {
@@ -457,6 +457,31 @@ public function testDownloadsToFileShouldNotCreateFileWhenObjectNotFound()
457457
$this->assertFileDoesNotExist($downloadFilePath);
458458
}
459459

460+
/**
461+
* @dataProvider provideInvalidFilePaths
462+
*/
463+
public function testDownloadsToFileShouldBlockPathTraversal(string $invalidFilePath)
464+
{
465+
$this->expectException(\RuntimeException::class);
466+
$this->expectExceptionMessage(
467+
'Path traversal is not allowed. File path is outside the designated directory.'
468+
);
469+
470+
$objectName = uniqid(self::TESTING_PREFIX);
471+
$testObject = self::$bucket->object($objectName);
472+
473+
$testObject->downloadToFile($invalidFilePath . $objectName);
474+
}
475+
476+
public function provideInvalidFilePaths()
477+
{
478+
return [
479+
['storage/../..'], // relative traversal
480+
['/storage/'], // absolute filepath
481+
['C:/storage/'], // windows drive letters
482+
];
483+
}
484+
460485
public function testDownloadsPublicFileWithUnauthenticatedClient()
461486
{
462487
$objectName = uniqid(self::TESTING_PREFIX);

Storage/tests/Unit/StorageObjectTest.php

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,7 @@ public function testDownloadsToFileShouldNotCreateFileWhenObjectNotFound()
540540
$this->connection->downloadObject(Argument::any())
541541
->willThrow(new NotFoundException($exceptionString));
542542
$object = 'non_existent_object.txt';
543-
$downloadFilePath = __DIR__ . '/storage_test_downloads_to_file.txt';
543+
$downloadFilePath = 'storage_test_downloads_to_file.txt';
544544
$object = new StorageObject($this->connection->reveal(), $object, self::BUCKET);
545545
$throws = false;
546546
try {
@@ -554,6 +554,33 @@ public function testDownloadsToFileShouldNotCreateFileWhenObjectNotFound()
554554
$this->assertFileDoesNotExist($downloadFilePath);
555555
}
556556

557+
/**
558+
* @dataProvider provideInvalidFilePaths
559+
*/
560+
public function testDownloadsToFileShouldBlockPathTraversal(string $invalidFilePath)
561+
{
562+
$this->expectException(\RuntimeException::class);
563+
$this->expectExceptionMessage(
564+
'Path traversal is not allowed. File path is outside the designated directory.'
565+
);
566+
$stream = Utils::streamFor('mock content');
567+
$this->connection->downloadObject(Argument::any())
568+
->willReturn($stream);
569+
$objectName = 'storage_test_downloads_to_file.txt';
570+
$object = new StorageObject($this->connection->reveal(), $objectName, self::BUCKET);
571+
572+
$object->downloadToFile($invalidFilePath . $objectName);
573+
}
574+
575+
public function provideInvalidFilePaths()
576+
{
577+
return [
578+
['storage/../..'], // relative traversal
579+
['/storage/'], // absolute filepath
580+
['C:/storage/'], // windows drive letters
581+
];
582+
}
583+
557584
public function testDownloadAsStreamWithoutExtraOptions()
558585
{
559586
$bucket = 'bucket';

0 commit comments

Comments
 (0)