fix(manager): close authorization gaps in document move, phpinfo and sysinfo

core/src/Controllers/MoveDocument.php

@@ -55,6 +55,9 @@ protected function handle()
 
         $document = $this->getDocument($documentID);
 
+        // the form (a=51) checks the source document; the action must too, or the check is bypassable
+        $this->checkDocumentPermission($document->getKey(), 'access_permission_denied');
+
         $parents = $this->managerTheme->getCore()->getParentIds($newParentID);
         if (\in_array($document->getKey(), $parents, true)) {
             $this->managerTheme->alertAndQuit('error_movedocument2');
@@ -121,15 +124,7 @@ protected function processDisplay() : bool
         $id = $this->getElementId();
         $document = $this->getDocument($id);
 
-        // check permissions on the document
-        $udperms = new Permissions();
-        $udperms->user     = $this->managerTheme->getCore()->getLoginUserID('mgr');
-        $udperms->document = $document->getKey();
-        $udperms->role     = $_SESSION['mgrRole'];
-
-        if (!$udperms->checkPermissions()) {
-            $this->managerTheme->alertAndQuit('access_permission_denied');
-        }
+        $this->checkDocumentPermission($document->getKey(), 'access_permission_denied');
 
         // Set the item name for logger
         $_SESSION['itemname'] = $document->pagetitle;
@@ -158,6 +153,16 @@ protected function getDocument($id) : Models\SiteContent
     }
 
     protected function checkNewParentPermission($id)
+    {
+        $this->checkDocumentPermission($id, 'access_permission_parent_denied');
+    }
+
+    /**
+     * @param int $id
+     * @param string $lang lexicon key of the alert shown when access is denied
+     * @since 3.5.8
+     */
+    protected function checkDocumentPermission($id, string $lang): void
     {
         $udperms           = new Permissions;
         $udperms->user     = $this->managerTheme->getCore()->getLoginUserID('mgr');
@@ -168,6 +173,6 @@ protected function checkNewParentPermission($id)
             return;
         }
 
-        $this->managerTheme->alertAndQuit('access_permission_parent_denied');
+        $this->managerTheme->alertAndQuit($lang);
     }
 }

core/src/Controllers/Phpinfo.php

@@ -21,7 +21,7 @@ public function checkLocked(): ?string
      */
     public function canView(): bool
     {
-        return $this->managerTheme->getCore()->hasPermission('logs');
+        return $this->managerTheme->getCore()->hasPermission('settings');
     }
 
     /**

core/src/Controllers/SystemInfo.php

@@ -27,7 +27,7 @@ public function checkLocked(): ?string
 
     public function canView(): bool
     {
-        return $this->managerTheme->getCore()->hasPermission('logs');
+        return $this->managerTheme->getCore()->hasPermission('settings');
     }
 
     public function getParameters(array $params = []): array

core/tests/Unit/Security/ManagerAuthorizationGapsTest.php

@@ -0,0 +1,44 @@
+<?php
+
+/*
+|--------------------------------------------------------------------------
+| Manager actions that used to check less than the page that leads to them
+|--------------------------------------------------------------------------
+|
+| The move form (a=51) enforces the per-document ACL, but the move action (a=52) it submits to
+| only checked the global save_document right, so the form was the only thing standing between a
+| user and a document outside their groups. Likewise phpinfo and the system info page expose the
+| server environment and the database layout, which is settings-level detail, but they were open
+| to anyone holding the much more common logs right.
+|
+*/
+
+function controllerSource(string $class): string
+{
+    return (string)file_get_contents(dirname(__DIR__, 3) . '/src/Controllers/' . $class . '.php');
+}
+
+it('checks the source document ACL in the move action, not only in the move form', function () {
+    $source = controllerSource('MoveDocument');
+
+    $handle = strpos($source, 'protected function handle()');
+    $display = strpos($source, 'protected function processDisplay()');
+    $check = strpos($source, "\$this->checkDocumentPermission(\$document->getKey(), 'access_permission_denied');", $handle);
+
+    expect($handle)->not->toBeFalse()
+        ->and($check)->not->toBeFalse()
+        ->and($check)->toBeLessThan($display)
+        ->and($check)->toBeLessThan(strpos($source, '$document->save();', $handle));
+
+    // the form keeps its own check
+    expect(strpos($source, "\$this->checkDocumentPermission(\$document->getKey(), 'access_permission_denied');", $display))
+        ->not->toBeFalse();
+});
+
+it('gates the phpinfo and system info pages on the settings right', function (string $class) {
+    $source = controllerSource($class);
+
+    expect($source)
+        ->toContain("hasPermission('settings')")
+        ->and($source)->not->toContain("hasPermission('logs')");
+})->with(['Phpinfo', 'SystemInfo']);

manager/views/frame/1.blade.php

@@ -361,6 +361,8 @@ function jsIconMarkup($icon) {
                                                 {{ icon_html($_style['icon_user_secret']) }} {{ManagerTheme::getLexicon('view_logging')}}
                                             </a>
                                         </li>
+                                    @endif
+                                    @if (evo()->hasPermission('settings'))
                                         <li>
                                             <a href="index.php?a=53" target="main">
                                                 {{ icon_html($_style['icon_info_circle']) }} {{ManagerTheme::getLexicon('view_sysinfo')}}