diff --git a/CHANGELOG.md b/CHANGELOG.md
index 94dfb5b70..1307b3347 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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)
@@ -30,6 +31,7 @@
* Fixed fatal error in `CompiledFile` if the cached version is broken
* Fixed updated media missing from media when editing Flex Object after page reload
* Fixed issue with `config-default@` breaking on set [#1972](https://github.com/getgrav/grav-plugin-admin/issues/1971)
+ * Escape titles in Flex pages list [flex-objects#84](https://github.com/trilbymedia/grav-plugin-flex-objects/issues/84)
# v1.7.0-rc.17
## 10/07/2020
diff --git a/system/blueprints/config/scheduler.yaml b/system/blueprints/config/scheduler.yaml
index c145045c7..caa771167 100644
--- a/system/blueprints/config/scheduler.yaml
+++ b/system/blueprints/config/scheduler.yaml
@@ -39,12 +39,13 @@ form:
.command:
type: text
label: PLUGIN_ADMIN.COMMAND
- placeholder: 'cd ~;ls -lah;'
+ placeholder: 'ls'
validate:
required: true
.args:
type: text
label: PLUGIN_ADMIN.EXTRA_ARGUMENTS
+ placeholder: '-lah'
.at:
type: cron
label: PLUGIN_ADMIN.SCHEDULER_RUNAT
diff --git a/system/blueprints/config/system.yaml b/system/blueprints/config/system.yaml
index de390e7fb..573fe2012 100644
--- a/system/blueprints/config/system.yaml
+++ b/system/blueprints/config/system.yaml
@@ -1343,6 +1343,12 @@ form:
label: PLUGIN_ADMIN.SESSION_PATH
help: PLUGIN_ADMIN.SESSION_PATH_HELP
+ session.samesite:
+ type: text
+ size: small
+ label: PLUGIN_ADMIN.SESSION_SAMESITE
+ help: PLUGIN_ADMIN.SESSION_SAMESITE_HELP
+
session.split:
type: toggle
label: PLUGIN_ADMIN.SESSION_SPLIT
diff --git a/system/config/system.yaml b/system/config/system.yaml
index 56f0a2e07..c8d8b04bc 100644
--- a/system/config/system.yaml
+++ b/system/config/system.yaml
@@ -168,6 +168,7 @@ session:
uniqueness: path # Should sessions be `path` based or `security.salt` based
secure: false # Set session secure. If true, indicates that communication for this cookie must be over an encrypted transmission. Enable this only on sites that run exclusively on HTTPS
httponly: true # Set session HTTP only. If true, indicates that cookies should be used only over HTTP, and JavaScript modification is not allowed.
+ samesite: # Set session SameSite. See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite
split: true # Sessions should be independent between site and plugins (such as admin)
path:
diff --git a/system/languages/en.yaml b/system/languages/en.yaml
index 7c351e649..7a8a68c00 100644
--- a/system/languages/en.yaml
+++ b/system/languages/en.yaml
@@ -94,9 +94,10 @@ GRAV:
YR_PLURAL: yrs
DEC_PLURAL: decs
FORM:
- VALIDATION_FAIL: Validation failed:
- INVALID_INPUT: Invalid input in
- MISSING_REQUIRED_FIELD: Missing required field:
+ VALIDATION_FAIL: 'Validation failed:'
+ 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"
diff --git a/system/src/Grav/Common/Data/Blueprint.php b/system/src/Grav/Common/Data/Blueprint.php
index 7755f9e54..a2f08dac2 100644
--- a/system/src/Grav/Common/Data/Blueprint.php
+++ b/system/src/Grav/Common/Data/Blueprint.php
@@ -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);
}
/**
diff --git a/system/src/Grav/Common/Data/BlueprintSchema.php b/system/src/Grav/Common/Data/BlueprintSchema.php
index 5c4a18860..e5c0d2389 100644
--- a/system/src/Grav/Common/Data/BlueprintSchema.php
+++ b/system/src/Grav/Common/Data/BlueprintSchema.php
@@ -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;
diff --git a/system/src/Grav/Common/Data/Validation.php b/system/src/Grav/Common/Data/Validation.php
index ad5448ce6..c69fb1918 100644
--- a/system/src/Grav/Common/Data/Validation.php
+++ b/system/src/Grav/Common/Data/Validation.php
@@ -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,72 @@ class Validation
return $messages;
}
+ /**
+ * @param mixed $value
+ * @param array $field
+ */
+ public static function checkSafety($value, array $field)
+ {
+ $messages = [];
+
+ $type = $field['validate']['type'] ?? $field['type'] ?? 'text';
+ if ($type === 'unset' || !($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.
*
diff --git a/system/src/Grav/Common/Flex/Types/Pages/PageIndex.php b/system/src/Grav/Common/Flex/Types/Pages/PageIndex.php
index 0e7e961d1..d97234067 100644
--- a/system/src/Grav/Common/Flex/Types/Pages/PageIndex.php
+++ b/system/src/Grav/Common/Flex/Types/Pages/PageIndex.php
@@ -489,7 +489,7 @@ class PageIndex extends FlexPageIndex implements PageCollectionInterface
$payload = [
'item-key' => basename($child->rawRoute() ?? $child->getKey()),
'icon' => $icon,
- 'title' => $child->menu(),
+ 'title' => htmlspecialchars($child->menu()),
'route' => [
'display' => $child->getRoute()->toString(false) ?: '/',
'raw' => $child->rawRoute(),
diff --git a/system/src/Grav/Common/Page/Pages.php b/system/src/Grav/Common/Page/Pages.php
index 4bb33212d..d61bad8a9 100644
--- a/system/src/Grav/Common/Page/Pages.php
+++ b/system/src/Grav/Common/Page/Pages.php
@@ -1079,7 +1079,7 @@ class Pages
$option = $current->route();
} else {
$extra = $showSlug ? '(' . $current->slug() . ') ' : '';
- $option = str_repeat('—-', $level). '▸ ' . $extra . $current->title();
+ $option = str_repeat('—-', $level). '▸ ' . $extra . htmlspecialchars($current->title());
}
$list[$route] = $option;
diff --git a/system/src/Grav/Common/Scheduler/Scheduler.php b/system/src/Grav/Common/Scheduler/Scheduler.php
index 942bfcb7c..a77b4e465 100644
--- a/system/src/Grav/Common/Scheduler/Scheduler.php
+++ b/system/src/Grav/Common/Scheduler/Scheduler.php
@@ -298,7 +298,7 @@ class Scheduler
*/
public function isCrontabSetup()
{
- $process = new Process('crontab -l');
+ $process = new Process(['crontab', '-l']);
$process->run();
if ($process->isSuccessful()) {
diff --git a/system/src/Grav/Common/Service/SessionServiceProvider.php b/system/src/Grav/Common/Service/SessionServiceProvider.php
index 2d4dffbc8..300d5e0fe 100644
--- a/system/src/Grav/Common/Service/SessionServiceProvider.php
+++ b/system/src/Grav/Common/Service/SessionServiceProvider.php
@@ -44,6 +44,7 @@ class SessionServiceProvider implements ServiceProviderInterface
$cookie_httponly = (bool)$config->get('system.session.httponly', true);
$cookie_lifetime = (int)$config->get('system.session.timeout', 1800);
$cookie_path = $config->get('system.session.path');
+ $cookie_samesite = $config->get('system.session.samesite');
if (null === $cookie_path) {
$cookie_path = '/' . trim(Uri::filterPath($uri->rootUrl(false)), '/');
}
@@ -95,7 +96,8 @@ class SessionServiceProvider implements ServiceProviderInterface
'cookie_path' => $cookie_path,
'cookie_domain' => $cookie_domain,
'cookie_secure' => $cookie_secure,
- 'cookie_httponly' => $cookie_httponly
+ 'cookie_httponly' => $cookie_httponly,
+ 'cookie_samesite' => $cookie_samesite
] + (array) $config->get('system.session.options');
$session = new Session($options);
diff --git a/system/src/Grav/Common/Utils.php b/system/src/Grav/Common/Utils.php
index 2213962fd..166bc37e7 100644
--- a/system/src/Grav/Common/Utils.php
+++ b/system/src/Grav/Common/Utils.php
@@ -928,11 +928,7 @@ abstract class Utils
public static function checkFilename($filename)
{
$dangerous_extensions = Grav::instance()['config']->get('security.uploads_dangerous_extensions', []);
- array_walk($dangerous_extensions, function (&$val) {
- $val = '.' . $val;
- });
-
- $extension = '.' . pathinfo($filename, PATHINFO_EXTENSION);
+ $extension = pathinfo($filename, PATHINFO_EXTENSION);
return !(
// Empty filenames are not allowed.
@@ -941,8 +937,8 @@ abstract class Utils
|| strtr($filename, "\t\v\n\r\0\\/", '_______') !== $filename
// Filename should not start or end with dot or space.
|| trim($filename, '. ') !== $filename
- // Filename should not contain .php in it.
- || static::contains($extension, $dangerous_extensions)
+ // File extension should not be part of configured dangerous extensions
+ || in_array($extension, $dangerous_extensions)
);
}
diff --git a/system/src/Grav/Framework/Flex/FlexObject.php b/system/src/Grav/Framework/Flex/FlexObject.php
index 2a271f021..449b117d4 100644
--- a/system/src/Grav/Framework/Flex/FlexObject.php
+++ b/system/src/Grav/Framework/Flex/FlexObject.php
@@ -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.
diff --git a/system/src/Grav/Framework/Session/Session.php b/system/src/Grav/Framework/Session/Session.php
index 8f7af7e82..20c41c04a 100644
--- a/system/src/Grav/Framework/Session/Session.php
+++ b/system/src/Grav/Framework/Session/Session.php
@@ -140,6 +140,7 @@ class Session implements SessionInterface
'use_strict_mode' => true,
'use_cookies' => true,
'use_only_cookies' => true,
+ 'cookie_samesite' => true,
'referer_check' => true,
'cache_limiter' => true,
'cache_expire' => true,
@@ -243,14 +244,19 @@ class Session implements SessionInterface
if ($sessionExists) {
$params = session_get_cookie_params();
+ $cookie_options = array (
+ 'expires' => time() + $params['lifetime'],
+ 'path' => $params['path'],
+ 'domain' => $params['domain'],
+ 'secure' => $params['secure'],
+ 'httponly' => $params['httponly'],
+ 'samesite' => $params['samesite']
+ );
+
setcookie(
$sessionName,
session_id(),
- time() + $params['lifetime'],
- $params['path'],
- $params['domain'],
- $params['secure'],
- $params['httponly']
+ $cookie_options
);
}
@@ -309,14 +315,20 @@ class Session implements SessionInterface
public function invalidate()
{
$params = session_get_cookie_params();
+
+ $cookie_options = array (
+ 'expires' => time() - 42000,
+ 'path' => $params['path'],
+ 'domain' => $params['domain'],
+ 'secure' => $params['secure'],
+ 'httponly' => $params['httponly'],
+ 'samesite' => $params['samesite']
+ );
+
setcookie(
session_name(),
'',
- time() - 42000,
- $params['path'],
- $params['domain'],
- $params['secure'],
- $params['httponly']
+ $cookie_options
);
if ($this->isSessionStarted()) {
diff --git a/tests/unit/Grav/Common/UtilsTest.php b/tests/unit/Grav/Common/UtilsTest.php
index 71ffb6f34..2b419080a 100644
--- a/tests/unit/Grav/Common/UtilsTest.php
+++ b/tests/unit/Grav/Common/UtilsTest.php
@@ -540,4 +540,20 @@ class UtilsTest extends \Codeception\TestCase\Test
$this->assertSame('//foo.com', Utils::url('//foo.com'));
$this->assertSame('//foo.com?param=x', Utils::url('//foo.com?param=x'));
}
+
+ public function testCheckFilename()
+ {
+ // configure extension for consistent results
+ /** @var \Grav\Common\Config\Config $config */
+ $config = $this->grav['config'];
+ $config->set('security.uploads_dangerous_extensions', ['php', 'html', 'htm', 'exe', 'js']);
+
+ $this->assertFalse(Utils::checkFilename('foo.php'));
+ $this->assertFalse(Utils::checkFilename('bar.js'));
+
+ $this->assertTrue(Utils::checkFilename('foo.json'));
+ $this->assertTrue(Utils::checkFilename('foo.xml'));
+ $this->assertTrue(Utils::checkFilename('foo.yaml'));
+ $this->assertTrue(Utils::checkFilename('foo.yml'));
+ }
}