Merge branch 'develop' of github.com:getgrav/grav-plugin-admin into 1.10

Conflicts: admin.php classes/plugin/AdminController.php composer.lock vendor/composer/autoload_classmap.php vendor/composer/autoload_psr4.php vendor/composer/autoload_real.php vendor/composer/autoload_static.php vendor/composer/installed.json
2025-11-02 03:16:11 +01:00 · 2020-12-01 09:24:05 +02:00
parent ddc85f3b89 9483b98be8
commit be6428922d
11 changed files with 131 additions and 67 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -315,6 +315,8 @@
 # v1.9.18
 ## mm/dd/2020

+1. [](#new)
+    * Never allow Admin pages to be rendered in `<frame>`, `<iframe>`, `<embed>` or `<object>` for improved security
 1. [](#improved)
    * Auto-link a plugin/theme license in details if it starts with `http`
    * Allow to fallback to `docs:` instead of `readme:`
@@ -322,6 +324,11 @@
    * Forward a `sid` to GPM when downloading a premium package
 1. [](#bugfix)
    * Escape page title in `pages` field
+    * Fixed unused task RemoveMedia, it cannot be used directly anymore [GHSA-945r-cjfm-642c](https://github.com/getgrav/grav-plugin-admin/security/advisories/GHSA-945r-cjfm-642c)
+    * Tightened checks when removing a media file [GHSA-945r-cjfm-642c](https://github.com/getgrav/grav-plugin-admin/security/advisories/GHSA-945r-cjfm-642c)
+    * Removed unused parameter in file field [GHSA-945r-cjfm-642c](https://github.com/getgrav/grav-plugin-admin/security/advisories/GHSA-945r-cjfm-642c)
+    * Fixed backup download URL [GHSA-vrvq-2pxg-rw5r](https://github.com/getgrav/grav-plugin-admin/security/advisories/GHSA-vrvq-2pxg-rw5r)
+    * Fixed deleting backup [GHSA-85r3-mf4x-qp8f](https://github.com/getgrav/grav-plugin-admin/security/advisories/GHSA-85r3-mf4x-qp8f)

 # v1.9.17
 ## 10/07/2020
--- a/admin.php
+++ b/admin.php
@@ -85,7 +85,7 @@ class AdminPlugin extends Plugin
                ['autoload', 100001],
                ['setup', 100000],
                ['onPluginsInitialized', 1001]
-              ],
+            ],
            'onRequestHandlerInit' => [
                ['onRequestHandlerInit', 100000]
            ],
@@ -383,7 +383,6 @@ class AdminPlugin extends Plugin
                // Create user object and save it
                $user = $users->load($username);
                $user->update($data);
-                $user->set('access', ['admin' => ['login' => true, 'super' => true], 'site' => ['login' => true]]);
                $user->save();

                // Login user
--- a/classes/Router.php
+++ b/classes/Router.php
@@ -0,0 +1,31 @@
+<?php
+
+namespace Grav\Plugin\Admin;
+
+use Grav\Common\Processors\ProcessorBase;
+use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\ServerRequestInterface;
+use Psr\Http\Server\RequestHandlerInterface;
+
+class Router extends ProcessorBase
+{
+    public $id = 'admin_router';
+    public $title = 'Admin Panel';
+
+    /**
+     * Admin router.
+     *
+     * @param ServerRequestInterface $request
+     * @param RequestHandlerInterface $handler
+     * @return ResponseInterface
+     */
+    public function process(ServerRequestInterface $request, RequestHandlerInterface $handler) : ResponseInterface
+    {
+        $this->startTimer();
+        $response = $handler->handle($request);
+        $this->stopTimer();
+
+        // Never allow admin pages to be rendered in <frame>, <iframe>, <embed> or <object> for improved security.
+        return $response->withHeader('X-Frame-Options', 'NONE');
+    }
+}
--- a/classes/plugin/AdminBaseController.php
+++ b/classes/plugin/AdminBaseController.php
@@ -920,11 +920,11 @@ class AdminBaseController
        $uri       = $this->grav['uri'];
        $blueprint = base64_decode($uri->param('blueprint'));
        $path      = base64_decode($uri->param('path'));
-        $filename  = basename($this->post['filename'] ?? '');
-        $proute    = base64_decode($uri->param('proute'));
+        $route     = base64_decode($uri->param('proute'));
        $type      = $uri->param('type');
        $field     = $uri->param('field');

+        $filename  = basename($this->post['filename'] ?? '');
        if ($filename === '') {
           $this->admin->json_response = [
                'status'  => 'error',
@@ -936,7 +936,7 @@ class AdminBaseController

        // Get Blueprint
        if ($type === 'pages' || strpos($blueprint, 'pages/') === 0) {
-            $page = $this->admin->page(true, $proute);
+            $page = $this->admin->page(true, $route);
            if (!$page) {
                $this->admin->json_response = [
                    'status'  => 'error',
@@ -1052,10 +1052,7 @@ class AdminBaseController
        }

        if (null === $filename) {
-            $filename = base64_decode($this->grav['uri']->param('route'));
-            if (!$filename) {
-                $filename = base64_decode($this->route);
-            }
+            throw new \RuntimeException('Admin task RemoveMedia has been disabled.');
        }

        $file                  = File::instance($filename);
--- a/classes/plugin/AdminController.php
+++ b/classes/plugin/AdminController.php
@@ -594,7 +594,7 @@ class AdminController extends AdminBaseController
    // BACKUP TASKS

    /**
-     * Handle the backup action
+     * Handle the backup action DEV
     *
     * @return bool True if the action was performed.
     */
@@ -609,13 +609,11 @@ class AdminController extends AdminBaseController

        try {
            if ($download) {
-                $file             = base64_decode(urldecode($download));
-                $backups_root_dir = $this->grav['locator']->findResource('backup://', true);
-
-                if (0 !== strpos($file, $backups_root_dir)) {
-                    $response = new Response(401);
-
-                    $this->close($response);
+                $filename = basename(base64_decode(urldecode($download)));
+                $file = $this->grav['locator']->findResource("backup://{$filename}", true);
+                if (!$file || !Utils::endsWith($filename, '.zip', false)) {
+                    header('HTTP/1.1 401 Unauthorized');
+                    exit();
                }

                Utils::download($file, true);
@@ -625,7 +623,7 @@ class AdminController extends AdminBaseController
            $backup = Backups::backup($id);
        } catch (\Exception $e) {
            $this->admin->json_response = [
-                'status'  => 'error',
+                'status' => 'error',
                'message' => $this->admin::translate('PLUGIN_ADMIN.AN_ERROR_OCCURRED') . '. ' . $e->getMessage()
            ];

@@ -633,18 +631,16 @@ class AdminController extends AdminBaseController
        }

        $download = urlencode(base64_encode($backup));
-        $url      = rtrim($this->grav['uri']->rootUrl(false), '/') . '/' . trim($this->admin->base,
+        $url = rtrim($this->grav['uri']->rootUrl(false), '/') . '/' . trim($this->admin->base,
                '/') . '/task' . $param_sep . 'backup/download' . $param_sep . $download . '/admin-nonce' . $param_sep . Utils::getNonce('admin-form');

-
-
        $this->admin->json_response = [
-            'status'  => 'success',
+            'status' => 'success',
            'message' => $this->admin::translate('PLUGIN_ADMIN.YOUR_BACKUP_IS_READY_FOR_DOWNLOAD') . '. <a href="' . $url . '" class="button">' . $this->admin::translate('PLUGIN_ADMIN.DOWNLOAD_BACKUP') . '</a>',
-            'toastr'  => [
-                'timeOut'         => 0,
+            'toastr' => [
+                'timeOut' => 0,
                'extendedTimeOut' => 0,
-                'closeButton'     => true
+                'closeButton' => true
            ]
        ];

@@ -658,7 +654,6 @@ class AdminController extends AdminBaseController
     */
    protected function taskBackupDelete()
    {
-        $param_sep = $this->grav['config']->get('system.param_sep', ':');
        if (!$this->authorizeTask('backup', ['admin.maintenance', 'admin.super'])) {
            return false;
        }
@@ -666,13 +661,11 @@ class AdminController extends AdminBaseController
        $backup = $this->grav['uri']->param('backup', null);

        if (null !== $backup) {
-            $file             = base64_decode(urldecode($backup));
-            $backups_root_dir = $this->grav['locator']->findResource('backup://', true);
+            $filename = basename(base64_decode(urldecode($backup)));
+            $file = $this->grav['locator']->findResource("backup://{$filename}", true);

-            $backup_path = $backups_root_dir . '/' . $file;
-
-            if (file_exists($backup_path)) {
-                unlink($backup_path);
+            if ($file && Utils::endsWith($filename, '.zip', false)) {
+                unlink($file);

                $this->admin->json_response = [
                    'status'  => 'success',
@@ -681,13 +674,16 @@ class AdminController extends AdminBaseController
                        'closeButton' => true
                    ]
                ];
-            } else {
-                $this->admin->json_response = [
-                    'status'  => 'error',
-                    'message' => $this->admin::translate('PLUGIN_ADMIN.BACKUP_NOT_FOUND'),
-                ];
+
+                return true;
            }
        }
+
+        $this->admin->json_response = [
+            'status'  => 'error',
+            'message' => $this->admin::translate('PLUGIN_ADMIN.BACKUP_NOT_FOUND'),
+        ];
+
        return true;
    }

@@ -1968,6 +1964,8 @@ class AdminController extends AdminBaseController
    /**
     * Determines the file types allowed to be uploaded
     *
+     * Used by pagemedia field.
+     *
     * @return bool True if the action was performed.
     */
    protected function taskListmedia()
@@ -2017,7 +2015,9 @@ class AdminController extends AdminBaseController
    }

    /**
-     * Handles adding a media file to a page
+     * Handles adding a media file to a page.
+     *
+     * Used by pagemedia field.
     *
     * @return bool True if the action was performed.
     */
@@ -2243,7 +2243,9 @@ class AdminController extends AdminBaseController
    }

    /**
-     * Handles deleting a media file from a page
+     * Handles deleting a media file from a page.
+     *
+     * Used by pagemedia field.
     *
     * @return bool True if the action was performed.
     */
@@ -2263,14 +2265,10 @@ class AdminController extends AdminBaseController
            return false;
        }

-        $filename = !empty($this->post['filename']) ? $this->post['filename'] : null;
+        $filename = !empty($this->post['filename']) ? basename($this->post['filename']) : null;

        // Handle bad filenames.
-        if (!Utils::checkFilename($filename)) {
-            $filename = null;
-        }
-
-        if (!$filename) {
+        if (!$filename || !Utils::checkFilename($filename)) {
            $this->admin->json_response = [
                'status'  => 'error',
                'message' => $this->admin::translate('PLUGIN_ADMIN.NO_FILE_FOUND')
@@ -2527,29 +2525,59 @@ class AdminController extends AdminBaseController
    }

    /**
-     * @return Media
+     * Get page media.
+     *
+     * @return Media|null
     */
-    protected function getMedia()
+    public function getMedia()
    {
-        $this->uri = $this->uri ?? $this->grav['uri'];
-        $uri = $this->uri->post('uri');
-        $order = $this->uri->post('order') ?: null;
-
-        if ($uri) {
-            /** @var UniformResourceLocator $locator */
-            $locator = $this->grav['locator'];
-
-            $media_path = $locator->isStream($uri) ? $uri : null;
-        } else {
-            $page = $this->admin->page(true);
-
-            $media_path = $page ? $page->path() : null;
+        if ($this->view !== 'media') {
+            return null;
        }
-        if ($order) {
+
+        $this->uri = $this->uri ?? $this->grav['uri'];
+        $this->grav['twig']->twig_vars['current_form_data'] = (array)$this->data;
+
+        $field = (string)$this->uri->post('field', '');
+        $order = $this->uri->post('order') ?: null;
+        if (!is_array($order)) {
            $order = array_map('trim', explode(',', $order));
        }

-        return $media_path ? new Media($media_path, $order) : null;
+        $page = $this->admin->page($this->route);
+        if (!$page) {
+            return null;
+        }
+
+        $blueprints = $page->blueprints();
+        $settings = $this->getMediaFieldSettings($blueprints, $field);
+        $path = $settings['destination'] ?? $page->path();
+
+        return $path ? new Media($path, $order) : null;
+    }
+
+    /**
+     * @param Data\Blueprint|null $blueprint
+     * @param string $field
+     * @return array|null
+     */
+    protected function getMediaFieldSettings(?Data\Blueprint $blueprint, string $field): ?array
+    {
+        $schema = $blueprint ? $blueprint->schema() : null;
+        if (!$schema || $field === '') {
+            return null;
+        }
+
+        $settings = is_object($schema) ? (array)$schema->getProperty($field) : null;
+        if (null === $settings) {
+            return null;
+        }
+
+        if (empty($settings['destination']) || \in_array($settings['destination'], ['@self', 'self@', '@self@'], true)) {
+            unset($settings['destination']);
+        }
+
+        return $settings + ['accept' => '*', 'limit' => 1000];
    }

    /**
--- a/languages/en.yaml
+++ b/languages/en.yaml
@@ -783,6 +783,7 @@ PLUGIN_ADMIN:
  SCHEDULER_OUTPUT_TYPE_HELP: "Either append to the same file each run, or overwrite the file with each run"
  SCHEDULER_EMAIL: "Email"
  SCHEDULER_EMAIL_HELP: "Email to send output to. NOTE: requires output file to be set"
+  SCHEDULER_WARNING: "The scheduler uses your system's crontab system to execute commands. You should use this only if you are an advanced user and know what you are doing. Misconfiguration or abuse can lead to security vulnerabilities."
  SECURITY: "Security"
  XSS_SECURITY: "XSS Security for Content"
  XSS_WHITELIST_PERMISSIONS: "Whitelist Permissions"
--- a/themes/grav/templates/forms/fields/backupshistory/backupshistory.html.twig
+++ b/themes/grav/templates/forms/fields/backupshistory/backupshistory.html.twig
@@ -19,7 +19,7 @@
            <td>{{ backup.title }}</td>
            <td class="right pad">{{ backup.size|nicefilesize }}</td>
            <td class="right pad nowrap" >
-                <a class="button button-small hint--bottom" href="{{ grav.backups.getBackupDownloadUrl(backup.path, admin.base) }}" data-hint="Download"><i class="fa fa-download"></i></a>
+                <a class="button button-small hint--bottom" href="{{ grav.backups.getBackupDownloadUrl(backup.filename, admin.base) }}" data-hint="Download"><i class="fa fa-download"></i></a>
                <span class="button button-small danger hint--bottom" data-hint="Delete" data-backup data-ajax="{{ backup_delete }}"><i class="fa fa-close"></i></span>
            </td>
        </tr>
--- a/themes/grav/templates/forms/fields/cronstatus/cronstatus.html.twig
+++ b/themes/grav/templates/forms/fields/cronstatus/cronstatus.html.twig
@@ -13,7 +13,7 @@
    <tbody>
    {% for job in jobs %}
        {% set job_status = attribute(data.status,job.id) %}
-        {% set job_enabled = job_status is defined and job_status == 'disabled' ? 0 : 1 %}
+        {% set job_enabled = job_status is defined and job_status != 'enabled' ? 0 : 1 %}
        {% set job_id = job.id %}
        {% set job_id_md5 = job_id|md5 %}
        {% set job_state = attribute(job_states, job_id) %}
--- a/themes/grav/templates/forms/fields/file/file.html.twig
+++ b/themes/grav/templates/forms/fields/file/file.html.twig
@@ -43,7 +43,6 @@
        {% set remove = global.file_task_remove ? global.file_url_remove : uri.addNonce(
            global.file_url_remove ~
            '/media.json' ~
-            '/route' ~ config.system.param_sep ~ base64_encode(global.base_path ~ '/' ~ real_path) ~
            '/task' ~ config.system.param_sep ~ 'removeFileFromBlueprint' ~
            '/proute' ~ config.system.param_sep ~ base64_encode(route) ~
            '/blueprint' ~ config.system.param_sep ~ blueprint ~
--- a/themes/grav/templates/partials/tools-scheduler.html.twig
+++ b/themes/grav/templates/partials/tools-scheduler.html.twig
@@ -15,6 +15,8 @@
        <div class="alert warning"> {{ "PLUGIN_ADMIN.SCHEDULER_NOT_ENABLED"|tu([user])|raw }}</div>
    {% endif %}

+    <div class="alert notice"><i class="fa fa-exclamation-circle"></i> {{ "PLUGIN_ADMIN.SCHEDULER_WARNING"|tu([user]) }}</div>
+
    <div id="cron-install" class="form-border overlay {{ cron_status == 1 ? 'hide' : ''}}">
        <pre><code>{{- grav.scheduler.getCronCommand()|trim -}}</code></pre>

--- a/vendor/composer/ClassLoader.php
+++ b/vendor/composer/ClassLoader.php
@@ -279,7 +279,7 @@ class ClassLoader
     */
    public function setApcuPrefix($apcuPrefix)
    {
-        $this->apcuPrefix = function_exists('apcu_fetch') && filter_var(ini_get('apc.enabled'), FILTER_VALIDATE_BOOLEAN) ? $apcuPrefix : null;
+        $this->apcuPrefix = function_exists('apcu_fetch') && ini_get('apc.enabled') ? $apcuPrefix : null;
    }

    /**
@@ -377,7 +377,7 @@ class ClassLoader
            $subPath = $class;
            while (false !== $lastPos = strrpos($subPath, '\\')) {
                $subPath = substr($subPath, 0, $lastPos);
-                $search = $subPath . '\\';
+                $search = $subPath.'\\';
                if (isset($this->prefixDirsPsr4[$search])) {
                    $pathEnd = DIRECTORY_SEPARATOR . substr($logicalPathPsr4, $lastPos + 1);
                    foreach ($this->prefixDirsPsr4[$search] as $dir) {