From ea0a5613cc40105787c3e6e9764c5c26a317c5e7 Mon Sep 17 00:00:00 2001 From: Andy Miller Date: Sun, 30 Sep 2018 15:24:29 -0600 Subject: [PATCH 1/4] XSS detection on header+content of page --- classes/admincontroller.php | 11 +++++++++-- languages/en.yaml | 2 +- themes/grav/templates/pages.html.twig | 1 + 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/classes/admincontroller.php b/classes/admincontroller.php index 83b93c2c..f4f11072 100644 --- a/classes/admincontroller.php +++ b/classes/admincontroller.php @@ -650,8 +650,15 @@ class AdminController extends AdminBaseController // XSS Checks for page content $xss_whitelist = $this->grav['config']->get('security.xss_whitelist', 'admin.super'); if (!$this->admin->authorize($xss_whitelist)) { - if ($issue = Utils::detectXss($data['content'])) { - $this->admin->setMessage(sprintf($this->admin->translate('PLUGIN_ADMIN.XSS_ISSUE'), $issue), + $check_what = ['header' => $data['header'], 'content' => $data['content']]; + $results = Utils::detectXssFromArray($check_what); + if (!empty($results)) { + $results_parts = array_map(function($value, $key) { + return $key.': \''.$value . '\''; + }, array_values($results), array_keys($results)); + + $output = implode(', ', $results_parts); + $this->admin->setMessage(' ' . sprintf($this->admin->translate('PLUGIN_ADMIN.XSS_ISSUE'), $output), 'error'); return false; } diff --git a/languages/en.yaml b/languages/en.yaml index d758afb1..e809d141 100644 --- a/languages/en.yaml +++ b/languages/en.yaml @@ -732,4 +732,4 @@ PLUGIN_ADMIN: XSS_RULES_HELP: "Be careful when tweaking these rules, a broken regex will break things badly!" XSS_RULE_LABEL: "Label" XSS_RULE_REGEX: "Regex" - XSS_ISSUE: "Save failed: Found potential XSS code of type: %s, please remove or disable the XSS filter in Configuration / Security." + XSS_ISSUE: "Save failed: Found potential XSS code in %s. Please remove or disable the XSS filter." diff --git a/themes/grav/templates/pages.html.twig b/themes/grav/templates/pages.html.twig index d1c0edf0..c9923980 100644 --- a/themes/grav/templates/pages.html.twig +++ b/themes/grav/templates/pages.html.twig @@ -322,6 +322,7 @@ {# Set current form data back into page content #} {% if current_form_data %} + {% do context.header(current_form_data.header) %} {% do context.content(current_form_data.content) %} {% endif %} {% if context.blueprints.fields and admin.session.expert == '0' %} From 3b5d3ca99a25d9455ba2ac4af05de03b5f1b4371 Mon Sep 17 00:00:00 2001 From: Andy Miller Date: Sun, 30 Sep 2018 15:25:39 -0600 Subject: [PATCH 2/4] format tidy --- classes/admincontroller.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/classes/admincontroller.php b/classes/admincontroller.php index f4f11072..5920929f 100644 --- a/classes/admincontroller.php +++ b/classes/admincontroller.php @@ -656,9 +656,7 @@ class AdminController extends AdminBaseController $results_parts = array_map(function($value, $key) { return $key.': \''.$value . '\''; }, array_values($results), array_keys($results)); - - $output = implode(', ', $results_parts); - $this->admin->setMessage(' ' . sprintf($this->admin->translate('PLUGIN_ADMIN.XSS_ISSUE'), $output), + $this->admin->setMessage(' ' . sprintf($this->admin->translate('PLUGIN_ADMIN.XSS_ISSUE'), implode(', ', $results_parts)), 'error'); return false; } From e0dc7de82709ce0dab53d0ba17bbb03fa9913cbd Mon Sep 17 00:00:00 2001 From: Andy Miller Date: Sun, 30 Sep 2018 15:26:19 -0600 Subject: [PATCH 3/4] valid frontmatter first --- classes/admincontroller.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/classes/admincontroller.php b/classes/admincontroller.php index 5920929f..742941e4 100644 --- a/classes/admincontroller.php +++ b/classes/admincontroller.php @@ -647,6 +647,13 @@ class AdminController extends AdminBaseController // Ensure route is prefixed with a forward slash. $route = '/' . ltrim($route, '/'); + // Check for valid frontmatter + if (isset($data['frontmatter']) && !$this->checkValidFrontmatter($data['frontmatter'])) { + $this->admin->setMessage($this->admin->translate('PLUGIN_ADMIN.INVALID_FRONTMATTER_COULD_NOT_SAVE'), + 'error'); + return false; + } + // XSS Checks for page content $xss_whitelist = $this->grav['config']->get('security.xss_whitelist', 'admin.super'); if (!$this->admin->authorize($xss_whitelist)) { @@ -662,13 +669,6 @@ class AdminController extends AdminBaseController } } - // Check for valid frontmatter - if (isset($data['frontmatter']) && !$this->checkValidFrontmatter($data['frontmatter'])) { - $this->admin->setMessage($this->admin->translate('PLUGIN_ADMIN.INVALID_FRONTMATTER_COULD_NOT_SAVE'), - 'error'); - return false; - } - $parent = $route && $route !== '/' && $route !== '.' && $route !== '/.' ? $pages->dispatch($route, true) : $pages->root(); $original_order = (int)trim($obj->order(), '.'); From bfc72d2d3e4c1de8d76d3e412b04d0c76c765192 Mon Sep 17 00:00:00 2001 From: Andy Miller Date: Sun, 30 Sep 2018 17:44:50 -0600 Subject: [PATCH 4/4] refactor class name --- classes/admincontroller.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/classes/admincontroller.php b/classes/admincontroller.php index 742941e4..a04ae000 100644 --- a/classes/admincontroller.php +++ b/classes/admincontroller.php @@ -14,6 +14,7 @@ use Grav\Common\Page\Medium\Medium; use Grav\Common\Page\Page; use Grav\Common\Page\Pages; use Grav\Common\Page\Collection; +use Grav\Common\Security; use Grav\Common\User\User; use Grav\Common\Utils; use Grav\Common\Backup\Backups; @@ -658,7 +659,7 @@ class AdminController extends AdminBaseController $xss_whitelist = $this->grav['config']->get('security.xss_whitelist', 'admin.super'); if (!$this->admin->authorize($xss_whitelist)) { $check_what = ['header' => $data['header'], 'content' => $data['content']]; - $results = Utils::detectXssFromArray($check_what); + $results = Security::detectXssFromArray($check_what); if (!empty($results)) { $results_parts = array_map(function($value, $key) { return $key.': \''.$value . '\'';