From 73f3d15bfb90ba779dd5498fcc5ae4ab292d6272 Mon Sep 17 00:00:00 2001
From: Steve Boyd <emteknetnz@gmail.com>
Date: Thu, 10 Nov 2022 15:25:53 +1300
Subject: [PATCH] [CVE-2022-42949] Subsite file permissions

---
 src/Extensions/FileSubsites.php |  6 ++--
 tests/php/FileSubsitesTest.php  | 61 +++++++++++++++++++++++++++++++++
 tests/php/SubsiteTest.yml       | 26 ++++++++++++++
 3 files changed, 90 insertions(+), 3 deletions(-)

diff --git a/src/Extensions/FileSubsites.php b/src/Extensions/FileSubsites.php
index 85defa9..39a7a96 100644
--- a/src/Extensions/FileSubsites.php
+++ b/src/Extensions/FileSubsites.php
@@ -116,9 +116,9 @@ class FileSubsites extends DataExtension
         }
 
         // Check the CMS_ACCESS_SecurityAdmin privileges on the subsite that owns this group
-        $subsiteID = SubsiteState::singleton()->getSubsiteId();
-        if ($subsiteID && $subsiteID == $this->owner->SubsiteID) {
-            return true;
+        $currentSubsiteID = SubsiteState::singleton()->getSubsiteId();
+        if ($currentSubsiteID && $currentSubsiteID !== $this->owner->SubsiteID) {
+            return false;
         }
 
         return SubsiteState::singleton()->withState(function (SubsiteState $newState) use ($member) {
diff --git a/tests/php/FileSubsitesTest.php b/tests/php/FileSubsitesTest.php
index 3aa6a6c..60f40c2 100644
--- a/tests/php/FileSubsitesTest.php
+++ b/tests/php/FileSubsitesTest.php
@@ -8,6 +8,7 @@ use SilverStripe\Core\Config\Config;
 use SilverStripe\Forms\FieldList;
 use SilverStripe\Subsites\Extensions\FileSubsites;
 use SilverStripe\Subsites\Model\Subsite;
+use SilverStripe\Security\Member;
 
 class FileSubsitesTest extends BaseSubsiteTest
 {
@@ -65,4 +66,64 @@ class FileSubsitesTest extends BaseSubsiteTest
         $file->onAfterUpload();
         $this->assertEquals($folder->SubsiteID, $file->SubsiteID);
     }
+
+    /**
+     * @dataProvider provideTestCanEdit
+     */
+    public function testCanEdit(
+        string $fileKey,
+        string $memberKey,
+        string $currentSubsiteKey,
+        bool $expected
+    ): void {
+        $file = $this->objFromFixture(File::class, $fileKey);
+        $subsiteID = ($currentSubsiteKey === 'mainsite')
+            ? 0 : $this->objFromFixture(Subsite::class, $currentSubsiteKey)->ID;
+        $member = $this->objFromFixture(Member::class, $memberKey);
+        Subsite::changeSubsite($subsiteID);
+        $this->assertSame($expected, $file->canEdit($member));
+    }
+
+    public function provideTestCanEdit(): array
+    {
+        $ret = [];
+        $data = [
+            // file
+            'subsite1file' => [
+                // member - has permissions to edit the file
+                'filetestyes' => [
+                    // current subite => expected canEdit()
+                    'subsite1' => true,
+                    'subsite2' => false,
+                    'mainsite' => true
+                ],
+                // member - does not have permissions to edit the file
+                'filetestno' => [
+                    'subsite1' => false,
+                    'subsite2' => false,
+                    'mainsite' => false
+                ],
+            ],
+            'mainsitefile' => [
+                'filetestyes' => [
+                    'subsite1' => true,
+                    'subsite2' => true,
+                    'mainsite' => true
+                ],
+                'filetestno' => [
+                    'subsite1' => false,
+                    'subsite2' => false,
+                    'mainsite' => false
+                ],
+            ]
+        ];
+        foreach (array_keys($data) as $fileKey) {
+            foreach (array_keys($data[$fileKey]) as $memberKey) {
+                foreach ($data[$fileKey][$memberKey] as $currentSubsiteKey => $expected) {
+                    $ret[] = [$fileKey, $memberKey, $currentSubsiteKey, $expected];
+                }
+            }
+        }
+        return $ret;
+    }
 }
diff --git a/tests/php/SubsiteTest.yml b/tests/php/SubsiteTest.yml
index 3096781..cc26248 100644
--- a/tests/php/SubsiteTest.yml
+++ b/tests/php/SubsiteTest.yml
@@ -159,6 +159,10 @@ SilverStripe\Security\Group:
     Code: subsite1_group_via_role
     AccessAllSubsites: 1
     Roles: =>SilverStripe\Security\PermissionRole.role1
+  filetest:
+    Title: filetest
+    Code: filetest
+    AccessAllSubsites: 1
 SilverStripe\Security\Permission:
   admin:
     Code: ADMIN
@@ -193,6 +197,9 @@ SilverStripe\Security\Permission:
   adminsubsite1:
     Code: ADMIN
     GroupID: =>SilverStripe\Security\Group.subsite1admins
+  filetest:
+    Code: CMS_ACCESS_CMSMain
+    GroupID: =>SilverStripe\Security\Group.filetest
 
 SilverStripe\Security\Member:
   admin:
@@ -222,7 +229,26 @@ SilverStripe\Security\Member:
   subsite1member2:
     Email: subsite1member2@test.com
     Groups: =>SilverStripe\Security\Group.subsite1_group_via_role
+  filetestyes:
+    Email: filetestyes@test.com
+    Groups: =>SilverStripe\Security\Group.filetest
+  filetestno:
+    Email: filetestno@test.com
 
 SilverStripe\SiteConfig\SiteConfig:
   config:
     CanCreateTopLevelType: LoggedInUsers
+
+SilverStripe\Assets\File:
+  subsite1file:
+    Name: subsitefile.pdf
+    Title: subsitefile
+    SubsiteID: =>SilverStripe\Subsites\Model\Subsite.subsite1
+    CanEditType: OnlyTheseUsers
+    EditorGroups: =>SilverStripe\Security\Group.filetest
+  mainsitefile:
+    Name: mainsitefile.pdf
+    Title: mainsitefile
+    SubsiteID: 0
+    CanEditType: OnlyTheseUsers
+    EditorGroups: =>SilverStripe\Security\Group.filetest