Prevent uploading .php files (#60)

* Prevent uploading .php files * Apply Code Style changes --------- Co-authored-by: duncanmcclean <[email protected]>
duncanmcclean · Nov 11, 2023 · a8e17b4 · a8e17b4
1 parent 3230046
commit a8e17b4
Show file tree

Hide file tree

Showing 6 changed files with 167 additions and 9 deletions.
diff --git a/src/Http/Controllers/GuestEntryController.php b/src/Http/Controllers/GuestEntryController.php
@@ -13,7 +13,9 @@
 use Illuminate\Http\Request;
 use Illuminate\Routing\Controller;
 use Illuminate\Support\Arr;
+use Illuminate\Support\Facades\Validator;
 use Illuminate\Support\Str;
+use Illuminate\Validation\ValidationException;
 use Statamic\Facades\Asset;
 use Statamic\Facades\AssetContainer;
 use Statamic\Facades\Collection;
@@ -240,8 +242,17 @@ protected function uploadFile(string $key, Field $field, Request $request)
             $uploadedFiles = [$uploadedFiles];
         }
 
-        // Filter out any null values.
-        $uploadedFiles = collect($uploadedFiles)->filter()->toArray();
+        $uploadedFiles = collect($uploadedFiles)
+            ->each(function ($file) use ($key) {
+                if (in_array(trim(strtolower($file->getClientOriginalExtension())), ['php', 'php3', 'php4', 'php5', 'phtml'])) {
+                    $validator = Validator::make([], []);
+                    $validator->errors()->add($key, __('Failed to upload.'));
+
+                    throw new ValidationException($validator);
+                }
+            })
+            ->filter()
+            ->toArray();
 
         /* @var \Illuminate\Http\Testing\File $file */
         foreach ($uploadedFiles as $uploadedFile) {

diff --git a/src/Http/Requests/DestroyRequest.php b/src/Http/Requests/DestroyRequest.php
@@ -8,8 +8,8 @@
 
 class DestroyRequest extends FormRequest
 {
-    use Concerns\WhitelistedCollections,
-        Concerns\HandleFailedValidation;
+    use Concerns\HandleFailedValidation,
+        Concerns\WhitelistedCollections;
 
     public function authorize()
     {

diff --git a/src/Http/Requests/StoreRequest.php b/src/Http/Requests/StoreRequest.php
@@ -9,8 +9,8 @@
 class StoreRequest extends FormRequest
 {
     use Concerns\AcceptsFormRequests,
-        Concerns\WhitelistedCollections,
-        Concerns\HandleFailedValidation;
+        Concerns\HandleFailedValidation,
+        Concerns\WhitelistedCollections;
 
     public function authorize()
     {

diff --git a/src/Http/Requests/UpdateRequest.php b/src/Http/Requests/UpdateRequest.php
@@ -9,8 +9,8 @@
 class UpdateRequest extends FormRequest
 {
     use Concerns\AcceptsFormRequests,
-        Concerns\WhitelistedCollections,
-        Concerns\HandleFailedValidation;
+        Concerns\HandleFailedValidation,
+        Concerns\WhitelistedCollections;
 
     public function authorize()
     {

diff --git a/tests/Http/Controllers/GuestEntryControllerTest.php b/tests/Http/Controllers/GuestEntryControllerTest.php
@@ -9,6 +9,7 @@
 use Illuminate\Support\Facades\Config;
 use Illuminate\Support\Facades\Event;
 use Illuminate\Support\Facades\File;
+use Illuminate\Support\Facades\Storage;
 use Spatie\TestTime\TestTime;
 use Statamic\Events\EntrySaved;
 use Statamic\Facades\Asset;
@@ -18,6 +19,8 @@
 use Statamic\Facades\Entry;
 use Statamic\Facades\Site;
 
+use function PHPUnit\Framework\assertCount;
+
 beforeEach(function () {
     File::deleteDirectory(app('stache')->store('entries')->directory());
 
@@ -462,6 +465,66 @@
     $this->assertIsString($entry->get('attachment'));
 });
 
+it('cant store an entry when uploading a PHP file', function () {
+    AssetContainer::make('assets')->disk('local')->save();
+
+    Blueprint::make('comments')
+        ->setNamespace('collections.comments')
+        ->setContents([
+            'title' => 'Comments',
+            'sections' => [
+                'main' => [
+                    'display' => 'main',
+                    'fields' => [
+                        [
+                            'handle' => 'title',
+                            'field' => [
+                                'type' => 'text',
+                            ],
+                        ],
+                        [
+                            'handle' => 'slug',
+                            'field' => [
+                                'type' => 'slug',
+                            ],
+                        ],
+                        [
+                            'handle' => 'attachment',
+                            'field' => [
+                                'mode' => 'list',
+                                'container' => 'assets',
+                                'restrict' => false,
+                                'allow_uploads' => true,
+                                'show_filename' => true,
+                                'display' => 'Attachment',
+                                'type' => 'assets',
+                                'icon' => 'assets',
+                                'listable' => 'hidden',
+                                'max_items' => 1,
+                            ],
+                        ],
+                    ],
+                ],
+            ],
+        ])
+        ->save();
+
+    Collection::make('comments')->save();
+
+    $this
+        ->post(route('statamic.guest-entries.store'), [
+            '_collection' => encrypt('comments'),
+            'title' => 'This is great',
+            'slug' => 'this-is-great',
+            'attachment' => UploadedFile::fake()->image('foobar.php'),
+        ])
+        ->assertSessionHasErrors('attachment');
+
+    assertCount(0, Entry::all());
+
+    Storage::disk('local')->assertMissing('assets/foobar.php');
+});
+
 it('can store entry and ensure multiple files can be uploaded', function () {
     AssetContainer::make('assets')->disk('local')->save();
 
@@ -1465,6 +1528,89 @@
     $this->assertIsString($entry->get('attachment'));
 });
 
+it('cant update entry when uploading a PHP file', function () {
+    AssetContainer::make('assets')->disk('local')->save();
+
+    Blueprint::make('albums')
+        ->setNamespace('collections.albums')
+        ->setContents([
+            'title' => 'Albums',
+            'sections' => [
+                'main' => [
+                    'display' => 'main',
+                    'fields' => [
+                        [
+                            'handle' => 'title',
+                            'field' => [
+                                'type' => 'text',
+                            ],
+                        ],
+                        [
+                            'handle' => 'artist',
+                            'field' => [
+                                'type' => 'text',
+                            ],
+                        ],
+                        [
+                            'handle' => 'slug',
+                            'field' => [
+                                'type' => 'slug',
+                            ],
+                        ],
+                        [
+                            'handle' => 'record_label',
+                            'field' => [
+                                'type' => 'text',
+                            ],
+                        ],
+                        [
+                            'handle' => 'attachment',
+                            'field' => [
+                                'mode' => 'list',
+                                'container' => 'assets',
+                                'restrict' => false,
+                                'allow_uploads' => true,
+                                'show_filename' => true,
+                                'display' => 'Attachment',
+                                'type' => 'assets',
+                                'icon' => 'assets',
+                                'listable' => 'hidden',
+                                'max_items' => 1,
+                            ],
+                        ],
+                    ],
+                ],
+            ],
+        ])
+        ->save();
+
+    Collection::make('albums')->save();
+
+    Entry::make()
+        ->id('allo-mate-idee')
+        ->collection('albums')
+        ->slug('allo-mate')
+        ->data([
+            'title' => 'Allo Mate!',
+            'artist' => 'Guvna B',
+        ])
+        ->save();
+
+    $this
+        ->post(route('statamic.guest-entries.update'), [
+            '_collection' => encrypt('albums'),
+            '_id' => encrypt('allo-mate-idee'),
+            'attachment' => UploadedFile::fake()->image('something.php'),
+        ])
+        ->assertSessionHasErrors('attachment');
+
+    $entry = Entry::find('allo-mate-idee');
+
+    $this->assertNull($entry->get('attachment'));
+
+    Storage::disk('local')->assertMissing('something.php');
+});
+
 it('can update entry and ensure multiple files can be uploaded', function () {
     AssetContainer::make('assets')->disk('local')->save();
 

diff --git a/tests/Tags/GuestEntriesTagTest.php b/tests/Tags/GuestEntriesTagTest.php
@@ -3,14 +3,15 @@
 use DuncanMcClean\GuestEntries\Tags\GuestEntriesTag;
 use Illuminate\Container\EntryNotFoundException;
 use Illuminate\Support\Facades\Config;
-use function PHPUnit\Framework\assertStringContainsString;
 use Statamic\Exceptions\CollectionNotFoundException;
 use Statamic\Facades\Antlers;
 use Statamic\Facades\Collection;
 use Statamic\Facades\Entry;
 use Statamic\Statamic;
 use Statamic\Tags\Tags;
 
+use function PHPUnit\Framework\assertStringContainsString;
+
 $tag = null;
 
 beforeEach(function () use (&$tag) {