CVE-2023-47621: Unrestricted Upload of File with Dangerous Type (CWE-434)
value: Guest Entries is a php library which allows users to create, update & delete entries from the front-end of a site. In affected versions the file uploads feature did not prevent the upload of PHP files. This may lead to code execution on the server by authenticated users. This vulnerability is fixed in v3.1.2. Users are advised to upgrade. There are no known workarounds for this vulnerability.
CWE description: The product allows the upload or transfer of dangerous file types that are automatically processed within its environment.
CVSS v3 base score: 8.8
Published: 2023-11-13T20:15Z
Repository: https://github.com/duncanmcclean/guest-entries
Language: PHP
Files changed: src/Http/Controllers/GuestEntryController.php, src/Http/Requests/DestroyRequest.php, src/Http/Requests/StoreRequest.php, src/Http/Requests/UpdateRequest.php, tests/Http/Controllers/GuestEntryControllerTest.php, tests/Tags/GuestEntriesTagTest.php
Commit date: 2023-11-11 10:31:54 +0000
Commit message: Prevent uploading .php files (#60)

* Prevent uploading .php files

* Apply Code Style changes

---------

Co-authored-by: duncanmcclean <duncanmcclean@users.noreply.github.com>
Diff: diff --git a/src/Http/Controllers/GuestEntryController.php b/src/Http/Controllers/GuestEntryController.php
index 362cfac..6827643 100644
--- a/src/Http/Controllers/GuestEntryController.php
+++ b/src/Http/Controllers/GuestEntryController.php
@@ -11,11 +11,13 @@ use DuncanMcClean\GuestEntries\Http\Requests\DestroyRequest;
 use DuncanMcClean\GuestEntries\Http\Requests\StoreRequest;
 use DuncanMcClean\GuestEntries\Http\Requests\UpdateRequest;
 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;
 use Statamic\Facades\Entry;
 use Statamic\Facades\Site as SiteFacade;
@@ -238,12 +240,21 @@ class GuestEntryController extends Controller
 
         if (! is_array($uploadedFiles)) {
             $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) {
             $path = '/'.$uploadedFile->storeAs(
                 isset($field->config()['folder'])
diff --git a/src/Http/Requests/DestroyRequest.php b/src/Http/Requests/DestroyRequest.php
index 3807021..5399c99 100644
--- a/src/Http/Requests/DestroyRequest.php
+++ b/src/Http/Requests/DestroyRequest.php
@@ -6,12 +6,12 @@ use DuncanMcClean\GuestEntries\Rules\CollectionExists;
 use DuncanMcClean\GuestEntries\Rules\EntryExists;
 use Illuminate\Foundation\Http\FormRequest;
 
 class DestroyRequest extends FormRequest
 {
-    use Concerns\WhitelistedCollections,
-        Concerns\HandleFailedValidation;
+    use Concerns\HandleFailedValidation,
+        Concerns\WhitelistedCollections;
 
     public function authorize()
     {
         return $this->collectionIsWhitelisted($this->get('_collection'));
     }
diff --git a/src/Http/Requests/StoreRequest.php b/src/Http/Requests/StoreRequest.php
index fe83e32..705f694 100644
--- a/src/Http/Requests/StoreRequest.php
+++ b/src/Http/Requests/StoreRequest.php
@@ -7,12 +7,12 @@ use Illuminate\Foundation\Http\FormRequest;
 use Statamic\Facades\Collection;
 
 class StoreRequest extends FormRequest
 {
     use Concerns\AcceptsFormRequests,
-        Concerns\WhitelistedCollections,
-        Concerns\HandleFailedValidation;
+        Concerns\HandleFailedValidation,
+        Concerns\WhitelistedCollections;
 
     public function authorize()
     {
         return $this->collectionIsWhitelisted($this->get('_collection'));
     }
diff --git a/src/Http/Requests/UpdateRequest.php b/src/Http/Requests/UpdateRequest.php
index e2db64c..3327d19 100644
--- a/src/Http/Requests/UpdateRequest.php
+++ b/src/Http/Requests/UpdateRequest.php
@@ -7,12 +7,12 @@ use DuncanMcClean\GuestEntries\Rules\EntryExists;
 use Illuminate\Foundation\Http\FormRequest;
 
 class UpdateRequest extends FormRequest
 {
     use Concerns\AcceptsFormRequests,
-        Concerns\WhitelistedCollections,
-        Concerns\HandleFailedValidation;
+        Concerns\HandleFailedValidation,
+        Concerns\WhitelistedCollections;
 
     public function authorize()
     {
         return $this->collectionIsWhitelisted($this->get('_collection'));
     }
diff --git a/tests/Http/Controllers/GuestEntryControllerTest.php b/tests/Http/Controllers/GuestEntryControllerTest.php
index 0b38574..55bce02 100644
--- a/tests/Http/Controllers/GuestEntryControllerTest.php
+++ b/tests/Http/Controllers/GuestEntryControllerTest.php
@@ -7,19 +7,22 @@ use DuncanMcClean\GuestEntries\Tests\Fixtures\FirstCustomStoreRequest;
 use DuncanMcClean\GuestEntries\Tests\Fixtures\FirstCustomUpdateRequest;
 use Illuminate\Http\UploadedFile;
 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;
 use Statamic\Facades\AssetContainer;
 use Statamic\Facades\Blueprint;
 use Statamic\Facades\Collection;
 use Statamic\Facades\Entry;
 use Statamic\Facades\Site;
 
+use function PHPUnit\Framework\assertCount;
+
 beforeEach(function () {
     File::deleteDirectory(app('stache')->store('entries')->directory());
 
     $this->app['config']->set('guest-entries.collections', [
         'comments' => true,
@@ -460,10 +463,70 @@ it('can store entry and ensure file can be uploaded', function () {
 
     $this->assertNotNull($entry->get('attachment'));
     $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();
 
     Blueprint::make('comments')
         ->setNamespace('collections.comments')
@@ -1463,10 +1526,93 @@ it('can update entry and ensure file can be uploaded', function () {
 
     $this->assertNotNull($entry->get('attachment'));
     $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();
 
     Blueprint::make('albums')
         ->setNamespace('collections.albums')
diff --git a/tests/Tags/GuestEntriesTagTest.php b/tests/Tags/GuestEntriesTagTest.php
index 2d9964c..091258e 100644
--- a/tests/Tags/GuestEntriesTagTest.php
+++ b/tests/Tags/GuestEntriesTagTest.php
@@ -1,18 +1,19 @@
 <?php
 
 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) {
     Config::set('guest-entries.disable_form_parameter_validation', true);
 

Vulnerable code:         // Filter out any null values.
        $uploadedFiles = collect($uploadedFiles)->filter()->toArray();
    use Concerns\WhitelistedCollections,
        Concerns\HandleFailedValidation;
        Concerns\WhitelistedCollections,
        Concerns\HandleFailedValidation;
        Concerns\WhitelistedCollections,
        Concerns\HandleFailedValidation;
use function PHPUnit\Framework\assertStringContainsString;
Fixed code: use Illuminate\Support\Facades\Validator;
use Illuminate\Validation\ValidationException;
        $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();
    use Concerns\HandleFailedValidation,
        Concerns\WhitelistedCollections;
        Concerns\HandleFailedValidation,
        Concerns\WhitelistedCollections;
        Concerns\HandleFailedValidation,
        Concerns\WhitelistedCollections;
use Illuminate\Support\Facades\Storage;
use function PHPUnit\Framework\assertCount;

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('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');
});

use function PHPUnit\Framework\assertStringContainsString;