Merge pull request #1654 from creative-commoners/pulls/1.13/cve-2023-…

…49783

[CVE-2023-49783] Opt in to permission checks in bulk loaders

code/GroupImportForm.php

@@ -89,6 +89,7 @@ public function __construct($controller, $name, $fields = null, $actions = null,
     public function doImport($data, $form)
     {
         $loader = new GroupCsvBulkLoader();
+        $loader->setCheckPermissions(true);
 
         // load file
         $result = $loader->load($data['CsvFile']['tmp_name']);

code/MemberImportForm.php

@@ -93,6 +93,7 @@ public function __construct($controller, $name, $fields = null, $actions = null,
     public function doImport($data, $form)
     {
         $loader = new MemberCsvBulkLoader();
+        $loader->setCheckPermissions(true);
 
         // optionally set group relation
         if ($this->group) {

code/ModelAdmin.php

@@ -6,6 +6,7 @@
 use SilverStripe\Control\Controller;
 use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Control\HTTPResponse;
+use SilverStripe\Control\HTTPResponse_Exception;
 use SilverStripe\Core\ClassInfo;
 use SilverStripe\Core\Convert;
 use SilverStripe\Core\Injector\Injector;
@@ -620,7 +621,11 @@ public function getModelImporters()
 
         $importers = [];
         foreach ($importerClasses as $modelClass => $importerClass) {
-            $importers[$modelClass] = new $importerClass($modelClass);
+            $importer = new $importerClass($modelClass);
+            if (ClassInfo::hasMethod($importer, 'setCheckPermissions')) {
+                $importer->setCheckPermissions(true);
+            }
+            $importers[$modelClass] = $importer;
         }
 
         return $importers;
@@ -647,19 +652,14 @@ public function ImportForm()
             return false;
         }
 
-        if (!$modelSNG->canCreate(Security::getCurrentUser())) {
-            return false;
-        }
-
         $fields = new FieldList(
             new HiddenField('ClassName', false, $this->modelClass),
             new FileField('_CsvFile', false)
         );
 
         // get HTML specification for each import (column names etc.)
-        $importerClass = $importers[$this->modelTab];
         /** @var BulkLoader $importer */
-        $importer = new $importerClass($this->modelClass);
+        $importer = $importers[$this->modelTab];
         $spec = $importer->getImportSpec();
         $specFields = new ArrayList();
         foreach ($spec['fields'] as $name => $desc) {
@@ -744,7 +744,13 @@ public function import($data, $form, $request)
         if (!empty($data['EmptyBeforeImport']) && $data['EmptyBeforeImport']) { //clear database before import
             $loader->deleteExistingRecords = true;
         }
-        $results = $loader->load($_FILES['_CsvFile']['tmp_name']);
+        try {
+            $results = $loader->load($_FILES['_CsvFile']['tmp_name']);
+        } catch (HTTPResponse_Exception $e) {
+            $form->sessionMessage($e->getMessage(), ValidationResult::TYPE_ERROR);
+            $this->redirectBack();
+            return false;
+        }
 
         $message = '';
 

composer.json

@@ -19,7 +19,7 @@
         }
     ],
     "require": {
-        "silverstripe/framework": "^4.10",
+        "silverstripe/framework": "^4.13.39",
         "silverstripe/versioned": "^1@dev",
         "silverstripe/vendor-plugin": "^1.0",
         "php": "^7.4 || ^8.0"
@@ -54,4 +54,4 @@
     },
     "minimum-stability": "dev",
     "prefer-stable": true
-}
+}

tests/behat/features/files/import-company-create.csv

@@ -0,0 +1,2 @@
+Name,Category,Revenue,CEO
+"New Company",Retail,123,"New person"

tests/behat/features/files/import-company-delete.csv

@@ -0,0 +1 @@
+ID,Name,Category,Revenue,CEO

tests/behat/features/files/import-company-edit.csv

@@ -0,0 +1,2 @@
+ID,Name,Category,Revenue,CEO
+1,"Some other company",Retail,123,"some person"

tests/behat/features/gridfield-import.feature

@@ -0,0 +1,83 @@
+Feature: Import in GridField
+  As a site owner
+  I want confidence that only users with permission can import records
+  So that I can sleep at night
+
+  Background:
+    Given the "Company" "Walmart" with "Category"="Retail"
+    And I am logged in with "ADMIN" permissions
+
+  Scenario: I can create new records via the import form
+    When I go to "/admin/test"
+    Then I should see 1 ".col-Name" elements
+    And I press the "Import CSV" button
+    Then I should see the "#Form_ImportForm" element
+    When I attach the file "import-company-create.csv" to the "_CsvFile" field
+    And I press the "Import from CSV" button
+    Then I should see "Imported 1 record" in the "#Form_ImportForm" element
+    And I should see 2 ".col-Name" elements
+
+  Scenario: I cannot create new records without permission
+    Given I add an extension "SilverStripe\Admin\Tests\Behat\Context\Extension\CannotCreateExtension" to the "SilverStripe\FrameworkTest\Model\Company" class
+    When I go to "/admin/test"
+    And I press the "Import CSV" button
+    Then I should see the "#Form_ImportForm" element
+    When I attach the file "import-company-create.csv" to the "_CsvFile" field
+    And I press the "Import from CSV" button
+    Then I should see "Not allowed to create 'Company' records" in the "#Form_ImportForm" element
+    And I should see 1 ".col-Name" elements
+    And I should see "Walmart" in the ".col-Name" element
+
+  Scenario: I can edit existing records via the import form
+    # To edit, we have to rely on IDs - so start by validating that a record with that ID exists at all.
+    When I go to "/admin/test/SilverStripe-FrameworkTest-Model-Company/EditForm/field/SilverStripe-FrameworkTest-Model-Company/item/1"
+    # Check we are viewing a record (we don't know or care what it's called)
+    Then I should see "Main" in the "div.cms-content-header-tabs" element
+    # Check we didn't get a not found error
+    And I should not see "Not Found"
+    When I go to "/admin/test"
+    And I press the "Import CSV" button
+    Then I should see the "#Form_ImportForm" element
+    When I attach the file "import-company-edit.csv" to the "_CsvFile" field
+    And I press the "Import from CSV" button
+    Then I should see "Updated 1 record" in the "#Form_ImportForm" element
+    And I should see 1 ".col-Name" elements
+    And I should not see "Walmart" in the ".col-Name" element
+    And I should see "Some other company" in the ".col-Name" element
+
+  Scenario: I cannot edit existing records without permission
+    Given I add an extension "SilverStripe\Admin\Tests\Behat\Context\Extension\CannotEditExtension" to the "SilverStripe\FrameworkTest\Model\Company" class
+    # We can't check if the record exists here because we don't have permission! But if it doesn't, this test will fail anyway.
+    When I go to "/admin/test"
+    And I press the "Import CSV" button
+    Then I should see the "#Form_ImportForm" element
+    When I attach the file "import-company-edit.csv" to the "_CsvFile" field
+    And I press the "Import from CSV" button
+    Then I should see "Not allowed to edit 'Company' records" in the "#Form_ImportForm" element
+    And I should see 1 ".col-Name" elements
+    And I should see "Walmart" in the ".col-Name" element
+    And I should not see "Some other company" in the ".col-Name" element
+
+  Scenario: I can delete records via the import form
+    When I go to "/admin/test"
+    And I press the "Import CSV" button
+    Then I should see the "#Form_ImportForm" element
+    When I attach the file "import-company-delete.csv" to the "_CsvFile" field
+    And I check "Replace data"
+    And I press the "Import from CSV" button
+    Then I should see "Nothing to import" in the "#Form_ImportForm" element
+    And I should see "No items found" in the ".ss-gridfield-items" element
+    And I should not see the ".col-Name" element
+
+  Scenario: I cannot delete records without permission
+    Given I add an extension "SilverStripe\Admin\Tests\Behat\Context\Extension\CannotDeleteExtension" to the "SilverStripe\FrameworkTest\Model\Company" class
+    When I go to "/admin/test"
+    And I press the "Import CSV" button
+    Then I should see the "#Form_ImportForm" element
+    When I attach the file "import-company-delete.csv" to the "_CsvFile" field
+    And I check "Replace data"
+    And I press the "Import from CSV" button
+    Then I should see "Not allowed to delete 'Company' records" in the "#Form_ImportForm" element
+    And I should not see "No items found" in the ".ss-gridfield-items" element
+    And I should see 1 ".col-Name" elements
+    And I should see "Walmart" in the ".col-Name" element

tests/behat/src/Extension/CannotCreateExtension.php

@@ -0,0 +1,14 @@
+<?php
+
+namespace SilverStripe\Admin\Tests\Behat\Context\Extension;
+
+use SilverStripe\Core\Extension;
+use SilverStripe\Dev\TestOnly;
+
+class CannotCreateExtension extends Extension implements TestOnly
+{
+    public function canCreate()
+    {
+        return false;
+    }
+}

tests/behat/src/Extension/CannotDeleteExtension.php

@@ -0,0 +1,14 @@
+<?php
+
+namespace SilverStripe\Admin\Tests\Behat\Context\Extension;
+
+use SilverStripe\Core\Extension;
+use SilverStripe\Dev\TestOnly;
+
+class CannotDeleteExtension extends Extension implements TestOnly
+{
+    public function canDelete()
+    {
+        return false;
+    }
+}

tests/behat/src/Extension/CannotEditExtension.php

@@ -0,0 +1,14 @@
+<?php
+
+namespace SilverStripe\Admin\Tests\Behat\Context\Extension;
+
+use SilverStripe\Core\Extension;
+use SilverStripe\Dev\TestOnly;
+
+class CannotEditExtension extends Extension implements TestOnly
+{
+    public function canEdit()
+    {
+        return false;
+    }
+}