Added XSS detection to all forms (use check_xss: false to disable it per field)

This commit is contained in:
Matias Griese 2020-11-17 18:54:03 +02:00
parent 11cd2b086e
commit ea00c044d3
6 changed files with 96 additions and 11 deletions

View File

@ -15,6 +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)
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)

View File

@ -94,9 +94,10 @@ GRAV:
YR_PLURAL: yrs
DEC_PLURAL: decs
FORM:
VALIDATION_FAIL: <b>Validation failed:</b>
INVALID_INPUT: Invalid input in
MISSING_REQUIRED_FIELD: Missing required field:
VALIDATION_FAIL: '<b>Validation failed:</b>'
INVALID_INPUT: 'Invalid input in'
MISSING_REQUIRED_FIELD: 'Missing required field:'
XSS_ISSUES: "Potential XSS issues detected in '%s' field"
MONTHS_OF_THE_YEAR: ['January', 'February', 'March', 'April', 'May', 'June', 'July', 'August', 'September', 'October', 'November', 'December']
DAYS_OF_THE_WEEK: ['Monday', 'Tuesday', 'Wednesday', 'Thursday', 'Friday', 'Saturday', 'Sunday']
YES: "Yes"

View File

@ -260,14 +260,15 @@ class Blueprint extends BlueprintForm
* Validate data against blueprints.
*
* @param array $data
* @param array $options
* @return void
* @throws RuntimeException
*/
public function validate(array $data)
public function validate(array $data, array $options = [])
{
$this->initInternals();
$this->blueprintSchema->validate($data);
$this->blueprintSchema->validate($data, $options);
}
/**

View File

@ -26,6 +26,9 @@ class BlueprintSchema extends BlueprintSchemaBase implements ExportInterface
{
use Export;
/** @var array */
protected $filter = ['validation' => true, 'check_xss' => true];
/** @var array */
protected $ignoreFormKeys = [
'title' => true,
@ -57,13 +60,14 @@ class BlueprintSchema extends BlueprintSchemaBase implements ExportInterface
* Validate data against blueprints.
*
* @param array $data
* @param array $options
* @throws RuntimeException
*/
public function validate(array $data)
public function validate(array $data, array $options = [])
{
try {
$validation = $this->items['']['form']['validation'] ?? 'loose';
$messages = $this->validateArray($data, $this->nested, $validation === 'strict');
$messages = $this->validateArray($data, $this->nested, $validation === 'strict', $options['check_xss'] ?? true);
} catch (RuntimeException $e) {
throw (new ValidationException($e->getMessage(), $e->getCode(), $e))->setMessages();
}
@ -141,16 +145,18 @@ class BlueprintSchema extends BlueprintSchemaBase implements ExportInterface
* @param array $data
* @param array $rules
* @param bool $strict
* @param bool $xss
* @return array
* @throws RuntimeException
*/
protected function validateArray(array $data, array $rules, bool $strict)
protected function validateArray(array $data, array $rules, bool $strict, bool $xss = true)
{
$messages = $this->checkRequired($data, $rules);
foreach ($data as $key => $child) {
$val = $rules[$key] ?? $rules['*'] ?? null;
$rule = is_string($val) ? $this->items[$val] : null;
$checkXss = $xss;
if ($rule) {
// Item has been defined in blueprints.
@ -160,11 +166,14 @@ class BlueprintSchema extends BlueprintSchemaBase implements ExportInterface
}
$messages += Validation::validate($child, $rule);
} elseif (is_array($child) && is_array($val)) {
// Array has been defined in blueprints.
$messages += $this->validateArray($child, $val, $strict);
$checkXss = false;
} elseif ($strict) {
// Undefined/extra item.
// Undefined/extra item in strict mode.
/** @var Config $config */
$config = Grav::instance()['config'];
if (!$config->get('system.strict_mode.blueprint_strict_compat', true)) {
@ -173,6 +182,10 @@ class BlueprintSchema extends BlueprintSchemaBase implements ExportInterface
user_error(sprintf('Having extra key %s in your data is deprecated with blueprint having \'validation: strict\'', $key), E_USER_DEPRECATED);
}
if ($checkXss) {
$messages += Validation::checkSafety($child, $rule ?: ['name' => $key]);
}
}
return $messages;

View File

@ -13,8 +13,12 @@ use ArrayAccess;
use Countable;
use DateTime;
use Grav\Common\Grav;
use Grav\Common\Language\Language;
use Grav\Common\Security;
use Grav\Common\User\Interfaces\UserInterface;
use Grav\Common\Utils;
use Grav\Common\Yaml;
use Grav\Framework\Flex\Interfaces\FlexObjectInterface;
use Traversable;
use function count;
use function is_array;
@ -92,6 +96,71 @@ class Validation
return $messages;
}
/**
* @param mixed $value
* @param array $field
*/
public static function checkSafety($value, array $field)
{
$messages = [];
if (!($field['check_xss'] ?? true)) {
return $messages;
}
$name = ucfirst($field['label'] ?? $field['name'] ?? 'UNKNOWN');
$user = Grav::instance()['user'] ?? null;
$xss_whitelist = Grav::instance()['config']->get('security.xss_whitelist', 'admin.super');
// Get language class.
/** @var Language $language */
$language = Grav::instance()['language'];
if (!static::authorize($xss_whitelist, $user)) {
if (is_string($value)) {
$violation = Security::detectXss($value);
if ($violation) {
$messages[$name][] = $language->translate(['GRAV.FORM.XSS_ISSUES', $language->translate($name)], null, true);
}
} elseif (is_array($value)) {
$violations = Security::detectXssFromArray($value, $name);
if ($violations) {
$messages[$name][] = $language->translate(['GRAV.FORM.XSS_ISSUES', $language->translate($name)], null, true);
}
}
}
return $messages;
}
/**
* Checks user authorisation to the action.
*
* @param string|string[] $action
* @param UserInterface|null $user
* @return bool
*/
public static function authorize($action, UserInterface $user = null)
{
if (!$user) {
return false;
}
$action = (array)$action;
foreach ($action as $a) {
// Ignore 'admin.super' if it's not the only value to be checked.
if ($a === 'admin.super' && count($action) > 1 && $user instanceof FlexObjectInterface) {
continue;
}
if ($user->authorize($a)) {
return true;
}
}
return false;
}
/**
* Filter value against a blueprint field definition.
*

View File

@ -134,7 +134,7 @@ class FlexObject implements FlexObjectInterface, FlexAuthorizeInterface
if ($validate) {
$blueprint = $this->getFlexDirectory()->getBlueprint();
$blueprint->validate($elements);
$blueprint->validate($elements, ['check_xss' => 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()]);
$blueprint->validate($test + ['storage_key' => $this->getStorageKey(), 'timestamp' => $this->getTimestamp()], ['check_xss' => false]);
$data = $blueprint->filter($data, true, true);
// Finally update the object.