From 400875b20f40e1343699d536a432a6fc284346da Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Mon, 18 Nov 2024 17:26:48 -0500 Subject: [PATCH] [5.x] More path traversal fixes (#11140) --- src/Assets/Asset.php | 8 ++++++++ tests/Assets/AssetContainerTest.php | 11 +++++++++++ tests/Assets/AssetFolderTest.php | 18 ++++++++++++++---- tests/Assets/AssetTest.php | 20 ++++++++++++++++++++ 4 files changed, 53 insertions(+), 4 deletions(-) diff --git a/src/Assets/Asset.php b/src/Assets/Asset.php index be11a4af78..c700ed7d7d 100644 --- a/src/Assets/Asset.php +++ b/src/Assets/Asset.php @@ -7,6 +7,7 @@ use Illuminate\Contracts\Support\Arrayable; use Illuminate\Support\Carbon; use Illuminate\Support\Facades\Cache; +use League\Flysystem\PathTraversalDetected; use Statamic\Assets\AssetUploader as Uploader; use Statamic\Contracts\Assets\Asset as AssetContract; use Statamic\Contracts\Assets\AssetContainer as AssetContainerContract; @@ -367,6 +368,13 @@ public function path($path = null) { return $this ->fluentlyGetOrSet('path') + ->setter(function ($path) { + if (str_contains($path, '../')) { + throw PathTraversalDetected::forPath($path); + } + + return $path; + }) ->getter(function ($path) { return $path ? ltrim($path, '/') : null; }) diff --git a/tests/Assets/AssetContainerTest.php b/tests/Assets/AssetContainerTest.php index 10ef45712f..507c735879 100644 --- a/tests/Assets/AssetContainerTest.php +++ b/tests/Assets/AssetContainerTest.php @@ -14,6 +14,7 @@ use League\Flysystem\DirectoryAttributes; use League\Flysystem\DirectoryListing; use League\Flysystem\FileAttributes; +use League\Flysystem\PathTraversalDetected; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use Statamic\Assets\Asset; @@ -811,6 +812,16 @@ public function it_makes_an_asset_at_given_path() $this->assertEquals('path/to/test.txt', $asset->path()); } + #[Test] + public function it_cannot_make_an_asset_using_path_traversal() + { + $this->expectException(PathTraversalDetected::class); + $this->expectExceptionMessage('Path traversal detected: foo/../test.txt'); + + $container = $this->containerWithDisk(); + $container->makeAsset('foo/../test.txt'); + } + #[Test] public function it_gets_all_assets_by_default() { diff --git a/tests/Assets/AssetFolderTest.php b/tests/Assets/AssetFolderTest.php index 55b10155d6..24a868a49e 100644 --- a/tests/Assets/AssetFolderTest.php +++ b/tests/Assets/AssetFolderTest.php @@ -57,12 +57,22 @@ public function it_gets_and_sets_the_path() } #[Test] - public function path_traversal_not_allowed() + public function it_cannot_use_traversal_in_path() { - $this->expectException(PathTraversalDetected::class); - $this->expectExceptionMessage('Path traversal detected: path/to/../folder'); + $folder = (new Folder)->path('path/to/folder'); - (new Folder)->path('path/to/../folder'); + try { + $folder->path('path/to/../folder'); + } catch (PathTraversalDetected $e) { + $this->assertEquals('Path traversal detected: path/to/../folder', $e->getMessage()); + + // Even if exception was thrown, make sure that the path didn't somehow get updated. + $this->assertEquals('path/to/folder', $folder->path()); + + return; + } + + $this->fail('Exception was not thrown.'); } #[Test] diff --git a/tests/Assets/AssetTest.php b/tests/Assets/AssetTest.php index 640e95f76c..06fc356c62 100644 --- a/tests/Assets/AssetTest.php +++ b/tests/Assets/AssetTest.php @@ -11,6 +11,7 @@ use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Storage; +use League\Flysystem\PathTraversalDetected; use Mockery; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; @@ -482,6 +483,25 @@ public function it_gets_and_sets_the_path() $this->assertEquals('asset.jpg', $asset->path('asset.jpg')->path()); } + #[Test] + public function it_cannot_use_traversal_in_path() + { + $asset = (new Asset)->path('path/to/asset.jpg'); + + try { + $asset->path('foo/../test.jpg'); + } catch (PathTraversalDetected $e) { + $this->assertEquals('Path traversal detected: foo/../test.jpg', $e->getMessage()); + + // Even if exception was thrown, make sure that the path didn't somehow get updated. + $this->assertEquals('path/to/asset.jpg', $asset->path()); + + return; + } + + $this->fail('Exception was not thrown.'); + } + #[Test] public function it_gets_the_id_from_the_container_and_path() {