From 91d8a16db237659938dd923fbaffcff1d95293bc Mon Sep 17 00:00:00 2001 From: Andy Miller Date: Mon, 1 Oct 2018 12:33:26 -0600 Subject: [PATCH] Refactored security --- system/blueprints/config/security.yaml | 88 ++++++++++++++++++-------- system/config/security.yaml | 48 +++++++------- system/src/Grav/Common/Security.php | 38 ++++++++--- 3 files changed, 115 insertions(+), 59 deletions(-) diff --git a/system/blueprints/config/security.yaml b/system/blueprints/config/security.yaml index c99e2effd..b309f1168 100644 --- a/system/blueprints/config/security.yaml +++ b/system/blueprints/config/security.yaml @@ -19,32 +19,66 @@ form: validate: type: commalist + xss_enabled.on_events: + type: toggle + label: PLUGIN_ADMIN.XSS_ON_EVENTS + highlight: 1 + options: + 1: PLUGIN_ADMIN.YES + 0: PLUGIN_ADMIN.NO + default: true + validate: + type: bool - xss_rules: - type: list - style: vertical - label: PLUGIN_ADMIN.XSS_RULES - help: PLUGIN_ADMIN.XSS_RULES_HELP - classes: compact - fields: - .label: - type: text - label: PLUGIN_ADMIN.XSS_RULE_LABEL - validate: - required: true - .regex: - type: text - label: PLUGIN_ADMIN.XSS_RULE_REGEX - validate: - required: true - .enabled: - type: toggle - label: PLUGIN_ADMIN.ENABLED - highlight: 1 - options: - 1: PLUGIN_ADMIN.YES - 0: PLUGIN_ADMIN.NO - default: true - validate: - type: bool + xss_enabled.invalid_protocols: + type: toggle + label: PLUGIN_ADMIN.XSS_INVALID_PROTOCOLS + highlight: 1 + options: + 1: PLUGIN_ADMIN.YES + 0: PLUGIN_ADMIN.NO + default: true + validate: + type: bool + + xss_enabled.moz_binding: + type: toggle + label: PLUGIN_ADMIN.XSS_MOZ_BINDINGS + highlight: 1 + options: + 1: PLUGIN_ADMIN.YES + 0: PLUGIN_ADMIN.NO + default: true + validate: + type: bool + + xss_enabled.html_inline_styles: + type: toggle + label: PLUGIN_ADMIN.XSS_HTML_INLINE_STYLES + highlight: 1 + options: + 1: PLUGIN_ADMIN.YES + 0: PLUGIN_ADMIN.NO + default: true + validate: + type: bool + + xss_enabled.dangerous_tags: + type: toggle + label: PLUGIN_ADMIN.XSS_DANGEROUS_TAGS + highlight: 1 + options: + 1: PLUGIN_ADMIN.YES + 0: PLUGIN_ADMIN.NO + default: true + validate: + type: bool + + xss_dangerous_tags: + type: selectize + size: large + label: PLUGIN_ADMIN.XSS_DANGEROUS_TAGS_LIST + classes: fancy + validate: + type: commalist diff --git a/system/config/security.yaml b/system/config/security.yaml index 69b44dd80..fce3e3f9b 100644 --- a/system/config/security.yaml +++ b/system/config/security.yaml @@ -1,22 +1,26 @@ -xss_whitelist: [admin.super] # Whitelist of user access that should 'skip' XSS checking -xss_rules: # Array of XSS tests to run through - - - label: On-Events - enabled: true - regex: '#(<[^>]+[\x00-\x20\"''\/])(on|xmlns)[^>]*>?#iUu' - - - label: JavaScript - enabled: true - regex: '!((java|live|vb)script|mocha|feed|data):(\w)*!iUu' - - - label: Moz-Binding - enabled: true - regex: '#-moz-binding[\x00-\x20]*:#u' - - - label: 'Style Attributes' - enabled: false - regex: '#(<[^>]+[\x00-\x20\"''\/])style=[^>]*>?#iUu' - - - label: 'Dangerous Tags' - enabled: true - regex: '#]*>?#ui' +xss_whitelist: [admin.super] # Whitelist of user access that should 'skip' XSS checking +xss_enabled: + on_events: true + invalid_protocols: true + moz_binding: true + html_inline_styles: true + dangerous_tags: true +xss_dangerous_tags: + - applet + - meta + - xml + - blink + - link + - style + - script + - embed + - object + - iframe + - frame + - frameset + - ilayer + - layer + - bgsound + - title + - base + diff --git a/system/src/Grav/Common/Security.php b/system/src/Grav/Common/Security.php index f7b247643..e8d3fd70c 100644 --- a/system/src/Grav/Common/Security.php +++ b/system/src/Grav/Common/Security.php @@ -116,20 +116,38 @@ class Security // Strip whitespace characters $string = preg_replace('!\s!u','', $string); - // Get XSS rules from security configuration - $xss_rules = Grav::instance()['config']->get('security.xss_rules'); + $config = Grav::instance()['config']; + + $dangerous_tags = $config->get('security.xss_dangerous_tags'); + $enabled_rules = $config->get('security.xss_enabled'); + + // Set the patterns we'll test against + $patterns = [ + // Match any attribute starting with "on" or xmlns + 'on_events' => '#(<[^>]+[[a-z\x00-\x20\"\'\/])(\son|\sxmlns)[a-z].*=>?#iUu', + + // Match javascript:, livescript:, vbscript:, mocha:, feed: and data: protocols + 'invalid_protocols' => '#((java|live|vb)script|mocha|feed|data):.*?#!iUu', + + // Match -moz-bindings + 'moz_binding' => '#-moz-binding[a-z\x00-\x20]*:#u', + + // Match style attributes + 'html_inline_styles' => '#(<[^>]+[a-z\x00-\x20\"\'\/])(style=[^>]*(url\:|x\:expression).*)>?#iUu', + + // Match potentially dangerous tags + 'dangerous_tags' => '#]*>?#ui' + ]; + // Iterate over rules and return label if fail - foreach ((array) $xss_rules as $rule) { - if ($rule['enabled'] === true) { - $label = $rule['label']; - $regex = $rule['regex']; + foreach ((array) $patterns as $name => $regex) { + if ($enabled_rules[$name] === true) { - if ($label && $regex) { - if (preg_match($regex, $string) || preg_match($regex, $orig)) { - return $label; - } + if (preg_match($regex, $string) || preg_match($regex, $orig)) { + return $name; } + } }