diff --git a/src/Assets/AssetFolder.php b/src/Assets/AssetFolder.php index 40932a1f20..050972efba 100644 --- a/src/Assets/AssetFolder.php +++ b/src/Assets/AssetFolder.php @@ -3,6 +3,7 @@ namespace Statamic\Assets; use Illuminate\Contracts\Support\Arrayable; +use League\Flysystem\PathTraversalDetected; use Statamic\Assets\AssetUploader as Uploader; use Statamic\Contracts\Assets\AssetFolder as Contract; use Statamic\Events\AssetFolderDeleted; @@ -35,7 +36,13 @@ public function container($container = null) public function path($path = null) { - return $this->fluentlyGetOrSet('path')->args(func_get_args()); + return $this->fluentlyGetOrSet('path') + ->setter(function ($path) { + if (str_contains($path, '..')) { + throw PathTraversalDetected::forPath($path); + } + }) + ->args(func_get_args()); } public function basename() diff --git a/src/Http/Controllers/CP/Assets/FolderActionController.php b/src/Http/Controllers/CP/Assets/FolderActionController.php index 32ac54fccc..0095cf10a6 100644 --- a/src/Http/Controllers/CP/Assets/FolderActionController.php +++ b/src/Http/Controllers/CP/Assets/FolderActionController.php @@ -2,7 +2,9 @@ namespace Statamic\Http\Controllers\CP\Assets; +use League\Flysystem\PathTraversalDetected; use Statamic\Assets\AssetFolder; +use Statamic\Exceptions\ValidationException; use Statamic\Http\Controllers\CP\ActionController as Controller; class FolderActionController extends Controller @@ -12,7 +14,11 @@ class FolderActionController extends Controller protected function getSelectedItems($items, $context) { return $items->map(function ($path) use ($context) { - return AssetFolder::find("{$context['container']}::{$path}"); + try { + return AssetFolder::find("{$context['container']}::{$path}"); + } catch (PathTraversalDetected $e) { + throw ValidationException::withMessages(['selections' => $e->getMessage()]); + } }); } } diff --git a/tests/Actions/DeleteAssetFolderTest.php b/tests/Actions/DeleteAssetFolderTest.php new file mode 100644 index 0000000000..ce7b52a48c --- /dev/null +++ b/tests/Actions/DeleteAssetFolderTest.php @@ -0,0 +1,101 @@ +container = tap( + (new AssetContainer)->handle('test_container')->disk('test') + )->save(); + } + + private function createAsset($filename) + { + $file = UploadedFile::fake()->image($filename, 30, 60); + Storage::disk('test')->putFileAs($file, $filename); + } + + private function assertAssetExists($file) + { + Storage::disk('test')->assertExists($file); + $this->assertNotNull($this->container->asset($file)); + } + + private function assertAssetDoesNotExist($file) + { + Storage::disk('test')->assertMissing($file); + $this->assertNull($this->container->asset($file)); + } + + private function deleteFolder($folder) + { + return $this->post(cp_route('assets.folders.actions.run', ['asset_container' => 'test_container']), [ + 'action' => 'delete', + 'context' => ['container' => 'test_container'], + 'selections' => [$folder], + 'values' => [], + ]); + } + + #[Test] + public function it_deletes() + { + $this->createAsset('foo/alfa.jpg'); + $this->createAsset('foo/bravo.jpg'); + $this->createAsset('bar/charlie.jpg'); + $this->createAsset('delta.jpg'); + Storage::disk('test')->assertExists('foo'); + + $this + ->actingAs(tap(User::make()->makeSuper())->save()) + ->deleteFolder('foo') + ->assertOk(); + + Storage::disk('test')->assertMissing('foo'); + $this->assertAssetDoesNotExist('foo/alfa.jpg'); + $this->assertAssetDoesNotExist('foo/bravo.jpg'); + $this->assertAssetExists('bar/charlie.jpg'); + $this->assertAssetExists('delta.jpg'); + } + + #[Test] + public function no_path_traversal() + { + $this->createAsset('foo/alfa.jpg'); + $this->createAsset('foo/bravo.jpg'); + $this->createAsset('bar/charlie.jpg'); + $this->createAsset('delta.jpg'); + Storage::disk('test')->assertExists('foo'); + + $this + ->actingAs(tap(User::make()->makeSuper())->save()) + ->deleteFolder('foo/..') + ->assertSessionHasErrors(['selections' => 'Path traversal detected: foo/..']); + + Storage::disk('test')->assertExists('foo'); + $this->assertAssetExists('foo/alfa.jpg'); + $this->assertAssetExists('foo/bravo.jpg'); + $this->assertAssetExists('bar/charlie.jpg'); + $this->assertAssetExists('delta.jpg'); + } +} diff --git a/tests/Assets/AssetFolderTest.php b/tests/Assets/AssetFolderTest.php index 3e181ea3bc..55b10155d6 100644 --- a/tests/Assets/AssetFolderTest.php +++ b/tests/Assets/AssetFolderTest.php @@ -8,6 +8,7 @@ use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Storage; use Illuminate\Support\Str; +use League\Flysystem\PathTraversalDetected; use PHPUnit\Framework\Attributes\Test; use Statamic\Assets\Asset; use Statamic\Assets\AssetContainerContents; @@ -55,6 +56,15 @@ public function it_gets_and_sets_the_path() $this->assertEquals('folder', $folder->basename()); } + #[Test] + public function path_traversal_not_allowed() + { + $this->expectException(PathTraversalDetected::class); + $this->expectExceptionMessage('Path traversal detected: path/to/../folder'); + + (new Folder)->path('path/to/../folder'); + } + #[Test] public function it_gets_the_disk_from_the_container() {