Nemcio/Grav
1
0
Fork 0
mirror of https://github.com/getgrav/grav.git synced 2026-07-19 23:50:43 +02:00
Code Issues Packages Projects Releases Activity

Add per field configuration options to customize XSS detection

Browse Source
This commit is contained in:
Matias Griese
2020-11-18 13:54:42 +02:00
parent 5f3ddd9389
commit 1f6c2f5a0a
5 changed files with 77 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
CHANGELOG.md
View File

@@ -15,7 +15,7 @@
* Allow `JsonFormatter` options to be passed as a string
* Hide Flex Pages frontend configuration (not ready for production use)
* Improve Flex configuration: gather views together in blueprint
* Added XSS detection to all forms (use `check_xss: false` to disable it per field)
* Added XSS detection to all forms. See [documentation](http://learn.grav.local/17/forms/forms/form-options#xss-checks)
1. [](#bugfix)
* *Menu Visibility Requires Access* Security option setting wrong frontmatter [login#265](https://github.com/getgrav/grav-plugin-login/issues/265)
* Accessing page with unsupported file extension (jpg, pdf, xsl) will use wrong mime type [#3031](https://github.com/getgrav/grav/issues/3031)

4
system/src/Grav/Common/Data/BlueprintSchema.php
View File

@@ -27,7 +27,7 @@ class BlueprintSchema extends BlueprintSchemaBase implements ExportInterface
use Export;
/** @var array */
protected $filter = ['validation' => true, 'check_xss' => true];
protected $filter = ['validation' => true, 'xss_check' => true];
/** @var array */
protected $ignoreFormKeys = [
@@ -67,7 +67,7 @@ class BlueprintSchema extends BlueprintSchemaBase implements ExportInterface
{
try {
$validation = $this->items['']['form']['validation'] ?? 'loose';
$messages = $this->validateArray($data, $this->nested, $validation === 'strict', $options['check_xss'] ?? true);
$messages = $this->validateArray($data, $this->nested, $validation === 'strict', $options['xss_check'] ?? true);
} catch (RuntimeException $e) {
throw (new ValidationException($e->getMessage(), $e->getCode(), $e))->setMessages();
}

28
system/src/Grav/Common/Data/Validation.php
View File

@@ -12,6 +12,7 @@ namespace Grav\Common\Data;
use ArrayAccess;
use Countable;
use DateTime;
use Grav\Common\Config\Config;
use Grav\Common\Grav;
use Grav\Common\Language\Language;
use Grav\Common\Security;
@@ -105,26 +106,45 @@ class Validation
$messages = [];
$type = $field['validate']['type'] ?? $field['type'] ?? 'text';
if ($type === 'unset' || !($field['check_xss'] ?? true)) {
$options = $field['xss_check'] ?? [];
if ($options === false || $type === 'unset') {
return $messages;
}
if (!is_array($options)) {
$options = [];
}
$name = ucfirst($field['label'] ?? $field['name'] ?? 'UNKNOWN');
/** @var UserInterface $user */
$user = Grav::instance()['user'] ?? null;
$xss_whitelist = Grav::instance()['config']->get('security.xss_whitelist', 'admin.super');
/** @var Config $config */
$config = Grav::instance()['config'];
$xss_whitelist = $config->get('security.xss_whitelist', 'admin.super');
// Get language class.
/** @var Language $language */
$language = Grav::instance()['language'];
if (!static::authorize($xss_whitelist, $user)) {
$defaults = Security::getXssDefaults();
$options += $defaults;
$options['enabled_rules'] += $defaults['enabled_rules'];
if (!empty($options['safe_protocols'])) {
$options['invalid_protocols'] = array_diff($options['invalid_protocols'], $options['safe_protocols']);
}
if (!empty($options['safe_tags'])) {
$options['dangerous_tags'] = array_diff($options['dangerous_tags'], $options['safe_tags']);
}
if (is_string($value)) {
$violation = Security::detectXss($value);
$violation = Security::detectXss($value, $options);
if ($violation) {
$messages[$name][] = $language->translate(['GRAV.FORM.XSS_ISSUES', $language->translate($name)], null, true);
}
} elseif (is_array($value)) {
$violations = Security::detectXssFromArray($value, $name);
$violations = Security::detectXssFromArray($value, "{$name}.", $options);
if ($violations) {
$messages[$name][] = $language->translate(['GRAV.FORM.XSS_ISSUES', $language->translate($name)], null, true);
}

67
system/src/Grav/Common/Security.php
View File

@@ -11,6 +11,7 @@ namespace Grav\Common;
use enshrined\svgSanitize\Sanitizer;
use Exception;
use Grav\Common\Config\Config;
use Grav\Common\Page\Pages;
use function chr;
use function count;
@@ -119,18 +120,22 @@ class Security
* Detect XSS in an array or strings such as $_POST or $_GET
*
* @param array $array Array such as $_POST or $_GET
* @param array|null $options Extra options to be passed.
* @param string $prefix Prefix for returned values.
* @return array Returns flatten list of potentially dangerous input values, such as 'data.content'.
*/
public static function detectXssFromArray(array $array, $prefix = '')
public static function detectXssFromArray(array $array, string $prefix = '', array $options = null)
{
$list = [];
if (null === $options) {
$options = static::getXssDefaults();
}
$list = [];
foreach ($array as $key => $value) {
if (is_array($value)) {
$list[] = static::detectXssFromArray($value, $prefix . $key . '.');
$list[] = static::detectXssFromArray($value, $prefix . $key . '.', $options);
}
if ($result = static::detectXss($value)) {
if ($result = static::detectXss($value, $options)) {
$list[] = [$prefix . $key => $result];
}
}
@@ -149,15 +154,34 @@ class Security
* their content.
*
* @param string|null $string The string to run XSS detection logic on
* @return bool|string Type of XSS vector if the given `$string` may contain XSS, false otherwise.
* @param array|null $options
* @return string|null Type of XSS vector if the given `$string` may contain XSS, false otherwise.
*
* Copies the code from: https://github.com/symphonycms/xssfilter/blob/master/extension.driver.php#L138
*/
public static function detectXss($string)
public static function detectXss($string, array $options = null): ?string
{
// Skip any null or non string values
if (null === $string || !is_string($string) || empty($string)) {
return false;
return null;
}
if (null === $options) {
$options = static::getXssDefaults();
}
$enabled_rules = (array)($options['enabled_rules'] ?? null);
$dangerous_tags = (array)($options['dangerous_tags'] ?? null);
if (!$dangerous_tags) {
$enabled_rules['dangerous_tags'] = false;
}
$invalid_protocols = (array)($options['invalid_protocols'] ?? null);
if (!$invalid_protocols) {
$enabled_rules['invalid_protocols'] = false;
}
$enabled_rules = array_filter($enabled_rules, static function ($val) { return !empty($val); });
if (!$enabled_rules) {
return null;
}
// Keep a copy of the original string before cleaning up
@@ -168,7 +192,7 @@ class Security
// Convert Hexadecimals
$string = (string)preg_replace_callback('!(&#|\\\)[xX]([0-9a-fA-F]+);?!u', function ($m) {
return \chr(hexdec($m[2]));
return chr(hexdec($m[2]));
}, $string);
// Clean up entities
@@ -180,19 +204,13 @@ class Security
// Strip whitespace characters
$string = preg_replace('!\s!u', '', $string);
$config = Grav::instance()['config'];
$dangerous_tags = array_map('preg_quote', array_map("trim", $config->get('security.xss_dangerous_tags')));
$invalid_protocols = array_map('preg_quote', array_map("trim", $config->get('security.xss_invalid_protocols')));
$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' => '#(' . implode('|', $invalid_protocols) . '):.*?#iUu',
'invalid_protocols' => '#(' . implode('|', array_map('preg_quote', $invalid_protocols, ['#'])) . '):.*?#iUu',
// Match -moz-bindings
'moz_binding' => '#-moz-binding[a-z\x00-\x20]*:#u',
@@ -201,13 +219,12 @@ class Security
'html_inline_styles' => '#(<[^>]+[a-z\x00-\x20\"\'\/])(style=[^>]*(url\:|x\:expression).*)>?#iUu',
// Match potentially dangerous tags
'dangerous_tags' => '#</*(' . implode('|', $dangerous_tags) . ')[^>]*>?#ui'
'dangerous_tags' => '#</*(' . implode('|', array_map('preg_quote', $dangerous_tags, ['#'])) . ')[^>]*>?#ui'
];
// Iterate over rules and return label if fail
foreach ((array) $patterns as $name => $regex) {
if ($enabled_rules[$name] === true) {
foreach ($patterns as $name => $regex) {
if (!empty($enabled_rules[$name])) {
if (preg_match($regex, $string) || preg_match($regex, $orig)) {
return $name;
}
@@ -216,4 +233,16 @@ class Security
return false;
}
public static function getXssDefaults(): array
{
/** @var Config $config */
$config = Grav::instance()['config'];
return [
'enabled_rules' => $config->get('security.xss_enabled'),
'dangerous_tags' => array_map('trim', $config->get('security.xss_dangerous_tags')),
'invalid_protocols' => array_map('trim', $config->get('security.xss_invalid_protocols')),
];
}
}

4
system/src/Grav/Framework/Flex/FlexObject.php
View File

@@ -134,7 +134,7 @@ class FlexObject implements FlexObjectInterface, FlexAuthorizeInterface
if ($validate) {
$blueprint = $this->getFlexDirectory()->getBlueprint();
$blueprint->validate($elements, ['check_xss' => false]);
$blueprint->validate($elements, ['xss_check' => false]);
$elements = $blueprint->filter($elements, true, true);
}
@@ -576,7 +576,7 @@ class FlexObject implements FlexObjectInterface, FlexAuthorizeInterface
$test = $blueprint->mergeData($elements, $data);
// Validate and filter elements and throw an error if any issues were found.
$blueprint->validate($test + ['storage_key' => $this->getStorageKey(), 'timestamp' => $this->getTimestamp()], ['check_xss' => false]);
$blueprint->validate($test + ['storage_key' => $this->getStorageKey(), 'timestamp' => $this->getTimestamp()], ['xss_check' => false]);
$data = $blueprint->filter($data, true, true);
// Finally update the object.