From be217f94e1c9ef021afe205034ffb8184ae61b55 Mon Sep 17 00:00:00 2001
From: bcweaver <brianweaver@gmail.com>
Date: Wed, 17 Oct 2018 20:13:47 -0400
Subject: [PATCH] SECURITY UPDATE Core 8.5.8
---
composer.json | 2 +-
composer.lock | 12 +-
vendor/composer/installed.json | 12 +-
web/core/lib/Drupal.php | 2 +-
.../Drupal/Component/Utility/UrlHelper.php | 10 +
.../RedirectResponseSubscriber.php | 32 --
.../Drupal/Core/Mail/Plugin/Mail/PhpMail.php | 48 ++-
.../Core/PathProcessor/PathProcessorAlias.php | 9 +
.../lib/Drupal/Core/Routing/UrlGenerator.php | 5 +
.../Drupal/Core/Security/RequestSanitizer.php | 13 +-
.../src/Functional/Views/DisplayBlockTest.php | 10 +-
.../Constraint/ModerationStateConstraint.php | 1 +
.../ModerationStateConstraintValidator.php | 90 ++++-
.../src/StateTransitionValidation.php | 10 +
.../StateTransitionValidationInterface.php | 19 +
.../Functional/ModerationStateNodeTest.php | 23 +-
.../EntityStateChangeValidationTest.php | 108 +++++
web/core/modules/contextual/contextual.module | 8 +-
.../contextual/contextual.post_update.php | 14 +
.../modules/contextual/js/contextual.es6.js | 20 +-
web/core/modules/contextual/js/contextual.js | 21 +-
.../contextual/src/ContextualController.php | 12 +-
.../Element/ContextualLinksPlaceholder.php | 11 +-
.../Tests/ContextualDynamicContextTest.php | 65 ++-
.../node/src/Tests/NodeRevisionsTest.php | 6 +
.../Tests/Views/NodeContextualLinksTest.php | 6 +
.../tests/src/Functional/PathAliasTest.php | 31 +-
.../system/src/Tests/Routing/RouterTest.php | 7 +
.../Tests/Component/Utility/UrlHelperTest.php | 4 +
.../RedirectResponseSubscriberTest.php | 71 ----
.../Tests/Core/Mail/MailManagerTest.php | 9 +
.../Core/Security/RequestSanitizerTest.php | 371 ++++++++++++++++++
32 files changed, 880 insertions(+), 182 deletions(-)
create mode 100644 web/core/modules/contextual/contextual.post_update.php
create mode 100644 web/core/tests/Drupal/Tests/Core/Security/RequestSanitizerTest.php
diff --git a/composer.json b/composer.json
index 88eea1561c..012361b5f0 100644
--- a/composer.json
+++ b/composer.json
@@ -83,7 +83,7 @@
"drupal/config_installer": "^1.0",
"drupal/console": "^1",
"drupal/content_access": "1.0-alpha1",
- "drupal/core": "8.5.7",
+ "drupal/core": "8.5.8",
"drupal/crop": "2.0-rc1",
"drupal/css_editor": "1.0",
"drupal/ctools": "3.0",
diff --git a/composer.lock b/composer.lock
index 6107cbce35..ce92533d8c 100644
--- a/composer.lock
+++ b/composer.lock
@@ -4,7 +4,7 @@
"Read more about it at https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file",
"This file is @generated automatically"
],
- "content-hash": "cdf06ca45889b346a4739bcd32760e82",
+ "content-hash": "1e26c9a92826825ab166954dbef61674",
"packages": [
{
"name": "alchemy/zippy",
@@ -2316,16 +2316,16 @@
},
{
"name": "drupal/core",
- "version": "8.5.7",
+ "version": "8.5.8",
"source": {
"type": "git",
"url": "https://github.com/drupal/core.git",
- "reference": "d8bdf376c0549e9bfc33030157b498df4b9bec87"
+ "reference": "01703d6f3995fcaa9c67bb02c524d7061dc040b4"
},
"dist": {
"type": "zip",
- "url": "https://api.github.com/repos/drupal/core/zipball/d8bdf376c0549e9bfc33030157b498df4b9bec87",
- "reference": "d8bdf376c0549e9bfc33030157b498df4b9bec87",
+ "url": "https://api.github.com/repos/drupal/core/zipball/01703d6f3995fcaa9c67bb02c524d7061dc040b4",
+ "reference": "01703d6f3995fcaa9c67bb02c524d7061dc040b4",
"shasum": ""
},
"require": {
@@ -2551,7 +2551,7 @@
"GPL-2.0-or-later"
],
"description": "Drupal is an open source content management platform powering millions of websites and applications.",
- "time": "2018-09-05T22:26:39+00:00"
+ "time": "2018-10-17T22:19:47+00:00"
},
{
"name": "drupal/crop",
diff --git a/vendor/composer/installed.json b/vendor/composer/installed.json
index 2c9fe51b24..d218a3cebe 100644
--- a/vendor/composer/installed.json
+++ b/vendor/composer/installed.json
@@ -2392,17 +2392,17 @@
},
{
"name": "drupal/core",
- "version": "8.5.7",
- "version_normalized": "8.5.7.0",
+ "version": "8.5.8",
+ "version_normalized": "8.5.8.0",
"source": {
"type": "git",
"url": "https://github.com/drupal/core.git",
- "reference": "d8bdf376c0549e9bfc33030157b498df4b9bec87"
+ "reference": "01703d6f3995fcaa9c67bb02c524d7061dc040b4"
},
"dist": {
"type": "zip",
- "url": "https://api.github.com/repos/drupal/core/zipball/d8bdf376c0549e9bfc33030157b498df4b9bec87",
- "reference": "d8bdf376c0549e9bfc33030157b498df4b9bec87",
+ "url": "https://api.github.com/repos/drupal/core/zipball/01703d6f3995fcaa9c67bb02c524d7061dc040b4",
+ "reference": "01703d6f3995fcaa9c67bb02c524d7061dc040b4",
"shasum": ""
},
"require": {
@@ -2571,7 +2571,7 @@
"symfony/debug": "^3.4.0",
"symfony/phpunit-bridge": "^3.4.3"
},
- "time": "2018-09-05T22:26:39+00:00",
+ "time": "2018-10-17T22:19:47+00:00",
"type": "drupal-core",
"extra": {
"merge-plugin": {
diff --git a/web/core/lib/Drupal.php b/web/core/lib/Drupal.php
index 901e77b370..b30f585da1 100644
--- a/web/core/lib/Drupal.php
+++ b/web/core/lib/Drupal.php
@@ -82,7 +82,7 @@ class Drupal {
/**
* The current system version.
*/
- const VERSION = '8.5.7';
+ const VERSION = '8.5.8';
/**
* Core API compatibility.
diff --git a/web/core/lib/Drupal/Component/Utility/UrlHelper.php b/web/core/lib/Drupal/Component/Utility/UrlHelper.php
index 372803590d..8f7019bf90 100644
--- a/web/core/lib/Drupal/Component/Utility/UrlHelper.php
+++ b/web/core/lib/Drupal/Component/Utility/UrlHelper.php
@@ -248,6 +248,16 @@ public static function isExternal($path) {
* Exception thrown when a either $url or $bath_url are not fully qualified.
*/
public static function externalIsLocal($url, $base_url) {
+ // Some browsers treat \ as / so normalize to forward slashes.
+ $url = str_replace('\\', '/', $url);
+
+ // Leading control characters may be ignored or mishandled by browsers, so
+ // assume such a path may lead to an non-local location. The \p{C} character
+ // class matches all UTF-8 control, unassigned, and private characters.
+ if (preg_match('/^\p{C}/u', $url) !== 0) {
+ return FALSE;
+ }
+
$url_parts = parse_url($url);
$base_parts = parse_url($base_url);
diff --git a/web/core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php b/web/core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php
index 8397bdef4e..67a4aae422 100644
--- a/web/core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php
+++ b/web/core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php
@@ -8,7 +8,6 @@
use Drupal\Core\Routing\RequestContext;
use Drupal\Core\Utility\UnroutedUrlAssemblerInterface;
use Symfony\Component\HttpFoundation\Response;
-use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\KernelEvents;
use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
use Symfony\Component\HttpFoundation\RedirectResponse;
@@ -129,36 +128,6 @@ protected function getDestinationAsAbsoluteUrl($destination, $scheme_and_host) {
return $destination;
}
- /**
- * Sanitize the destination parameter to prevent open redirect attacks.
- *
- * @param \Symfony\Component\HttpKernel\Event\GetResponseEvent $event
- * The Event to process.
- */
- public function sanitizeDestination(GetResponseEvent $event) {
- $request = $event->getRequest();
- // Sanitize the destination parameter (which is often used for redirects) to
- // prevent open redirect attacks leading to other domains. Sanitize both
- // $_GET['destination'] and $_REQUEST['destination'] to protect code that
- // relies on either, but do not sanitize $_POST to avoid interfering with
- // unrelated form submissions. The sanitization happens here because
- // url_is_external() requires the variable system to be available.
- $query_info = $request->query;
- $request_info = $request->request;
- if ($query_info->has('destination') || $request_info->has('destination')) {
- // If the destination is an external URL, remove it.
- if ($query_info->has('destination') && UrlHelper::isExternal($query_info->get('destination'))) {
- $query_info->remove('destination');
- $request_info->remove('destination');
- }
- // If there's still something in $_REQUEST['destination'] that didn't come
- // from $_GET, check it too.
- if ($request_info->has('destination') && (!$query_info->has('destination') || $request_info->get('destination') != $query_info->get('destination')) && UrlHelper::isExternal($request_info->get('destination'))) {
- $request_info->remove('destination');
- }
- }
- }
-
/**
* Registers the methods in this class that should be listeners.
*
@@ -167,7 +136,6 @@ public function sanitizeDestination(GetResponseEvent $event) {
*/
public static function getSubscribedEvents() {
$events[KernelEvents::RESPONSE][] = ['checkRedirectUrl'];
- $events[KernelEvents::REQUEST][] = ['sanitizeDestination', 100];
return $events;
}
diff --git a/web/core/lib/Drupal/Core/Mail/Plugin/Mail/PhpMail.php b/web/core/lib/Drupal/Core/Mail/Plugin/Mail/PhpMail.php
index e5361492b5..27e5c76e5d 100644
--- a/web/core/lib/Drupal/Core/Mail/Plugin/Mail/PhpMail.php
+++ b/web/core/lib/Drupal/Core/Mail/Plugin/Mail/PhpMail.php
@@ -18,6 +18,20 @@
*/
class PhpMail implements MailInterface {
+ /**
+ * The configuration factory.
+ *
+ * @var \Drupal\Core\Config\ConfigFactoryInterface
+ */
+ protected $configFactory;
+
+ /**
+ * PhpMail constructor.
+ */
+ public function __construct() {
+ $this->configFactory = \Drupal::configFactory();
+ }
+
/**
* Concatenates and wraps the email body for plain-text mails.
*
@@ -86,7 +100,10 @@ public function mail(array $message) {
// On most non-Windows systems, the "-f" option to the sendmail command
// is used to set the Return-Path. There is no space between -f and
// the value of the return path.
- $additional_headers = isset($message['Return-Path']) ? '-f' . $message['Return-Path'] : '';
+ // We validate the return path, unless it is equal to the site mail, which
+ // we assume to be safe.
+ $site_mail = $this->configFactory->get('system.site')->get('mail');
+ $additional_headers = isset($message['Return-Path']) && ($site_mail === $message['Return-Path'] || static::_isShellSafe($message['Return-Path'])) ? '-f' . $message['Return-Path'] : '';
$mail_result = @mail(
$message['to'],
$mail_subject,
@@ -112,4 +129,33 @@ public function mail(array $message) {
return $mail_result;
}
+ /**
+ * Disallows potentially unsafe shell characters.
+ *
+ * Functionally similar to PHPMailer::isShellSafe() which resulted from
+ * CVE-2016-10045. Note that escapeshellarg and escapeshellcmd are inadequate
+ * for this purpose.
+ *
+ * @param string $string
+ * The string to be validated.
+ *
+ * @return bool
+ * True if the string is shell-safe.
+ *
+ * @see https://github.com/PHPMailer/PHPMailer/issues/924
+ * @see https://github.com/PHPMailer/PHPMailer/blob/v5.2.21/class.phpmailer.php#L1430
+ *
+ * @todo Rename to ::isShellSafe() and/or discuss whether this is the correct
+ * location for this helper.
+ */
+ protected static function _isShellSafe($string) {
+ if (escapeshellcmd($string) !== $string || !in_array(escapeshellarg($string), ["'$string'", "\"$string\""])) {
+ return FALSE;
+ }
+ if (preg_match('/[^a-zA-Z0-9@_\-.]/', $string) !== 0) {
+ return FALSE;
+ }
+ return TRUE;
+ }
+
}
diff --git a/web/core/lib/Drupal/Core/PathProcessor/PathProcessorAlias.php b/web/core/lib/Drupal/Core/PathProcessor/PathProcessorAlias.php
index b85737f3cf..d0690fee20 100644
--- a/web/core/lib/Drupal/Core/PathProcessor/PathProcessorAlias.php
+++ b/web/core/lib/Drupal/Core/PathProcessor/PathProcessorAlias.php
@@ -43,6 +43,15 @@ public function processOutbound($path, &$options = [], Request $request = NULL,
if (empty($options['alias'])) {
$langcode = isset($options['language']) ? $options['language']->getId() : NULL;
$path = $this->aliasManager->getAliasByPath($path, $langcode);
+ // Ensure the resulting path has at most one leading slash, to prevent it
+ // becoming an external URL without a protocol like //example.com. This
+ // is done in \Drupal\Core\Routing\UrlGenerator::generateFromRoute()
+ // also, to protect against this problem in arbitrary path processors,
+ // but it is duplicated here to protect any other URL generation code
+ // that might call this method separately.
+ if (strpos($path, '//') === 0) {
+ $path = '/' . ltrim($path, '/');
+ }
}
return $path;
}
diff --git a/web/core/lib/Drupal/Core/Routing/UrlGenerator.php b/web/core/lib/Drupal/Core/Routing/UrlGenerator.php
index 3b5c8d256b..853a5f7b4e 100644
--- a/web/core/lib/Drupal/Core/Routing/UrlGenerator.php
+++ b/web/core/lib/Drupal/Core/Routing/UrlGenerator.php
@@ -297,6 +297,11 @@ public function generateFromRoute($name, $parameters = [], $options = [], $colle
if ($options['path_processing']) {
$path = $this->processPath($path, $options, $generated_url);
}
+ // Ensure the resulting path has at most one leading slash, to prevent it
+ // becoming an external URL without a protocol like //example.com.
+ if (strpos($path, '//') === 0) {
+ $path = '/' . ltrim($path, '/');
+ }
// The contexts base URL is already encoded
// (see Symfony\Component\HttpFoundation\Request).
$path = str_replace($this->decodedChars[0], $this->decodedChars[1], rawurlencode($path));
diff --git a/web/core/lib/Drupal/Core/Security/RequestSanitizer.php b/web/core/lib/Drupal/Core/Security/RequestSanitizer.php
index 3f48f3d59c..e1626ed383 100644
--- a/web/core/lib/Drupal/Core/Security/RequestSanitizer.php
+++ b/web/core/lib/Drupal/Core/Security/RequestSanitizer.php
@@ -90,7 +90,8 @@ protected static function processParameterBag(ParameterBag $bag, $whitelist, $lo
}
if ($bag->has('destination')) {
- $destination_dangerous_keys = static::checkDestination($bag->get('destination'), $whitelist);
+ $destination = $bag->get('destination');
+ $destination_dangerous_keys = static::checkDestination($destination, $whitelist);
if (!empty($destination_dangerous_keys)) {
// The destination is removed rather than sanitized because the URL
// generator service is not available and this method is called very
@@ -101,6 +102,16 @@ protected static function processParameterBag(ParameterBag $bag, $whitelist, $lo
trigger_error(sprintf('Potentially unsafe destination removed from %s parameter bag because it contained the following keys: %s', $bag_name, implode(', ', $destination_dangerous_keys)));
}
}
+ // Sanitize the destination parameter (which is often used for redirects)
+ // to prevent open redirect attacks leading to other domains.
+ if (UrlHelper::isExternal($destination)) {
+ // The destination is removed because it is an external URL.
+ $bag->remove('destination');
+ $sanitized = TRUE;
+ if ($log_sanitized_keys) {
+ trigger_error(sprintf('Potentially unsafe destination removed from %s parameter bag because it points to an external URL.', $bag_name));
+ }
+ }
}
return $sanitized;
}
diff --git a/web/core/modules/block/tests/src/Functional/Views/DisplayBlockTest.php b/web/core/modules/block/tests/src/Functional/Views/DisplayBlockTest.php
index 0a18d7fa0f..cdb1998241 100644
--- a/web/core/modules/block/tests/src/Functional/Views/DisplayBlockTest.php
+++ b/web/core/modules/block/tests/src/Functional/Views/DisplayBlockTest.php
@@ -3,6 +3,8 @@
namespace Drupal\Tests\block\Functional\Views;
use Drupal\Component\Serialization\Json;
+use Drupal\Component\Utility\Crypt;
+use Drupal\Core\Site\Settings;
use Drupal\Core\Url;
use Drupal\Tests\block\Functional\AssertBlockAppearsTrait;
use Drupal\Tests\system\Functional\Cache\AssertPageCacheContextsAndTagsTrait;
@@ -360,14 +362,16 @@ public function testBlockContextualLinks() {
$this->drupalGet('test-page');
$id = 'block:block=' . $block->id() . ':langcode=en|entity.view.edit_form:view=test_view_block:location=block&name=test_view_block&display_id=block_1&langcode=en';
+ $id_token = Crypt::hmacBase64($id, Settings::getHashSalt() . $this->container->get('private_key')->get());
$cached_id = 'block:block=' . $cached_block->id() . ':langcode=en|entity.view.edit_form:view=test_view_block:location=block&name=test_view_block&display_id=block_1&langcode=en';
+ $cached_id_token = Crypt::hmacBase64($cached_id, Settings::getHashSalt() . $this->container->get('private_key')->get());
// @see \Drupal\contextual\Tests\ContextualDynamicContextTest:assertContextualLinkPlaceHolder()
- $this->assertRaw('<div' . new Attribute(['data-contextual-id' => $id]) . '></div>', format_string('Contextual link placeholder with id @id exists.', ['@id' => $id]));
- $this->assertRaw('<div' . new Attribute(['data-contextual-id' => $cached_id]) . '></div>', format_string('Contextual link placeholder with id @id exists.', ['@id' => $cached_id]));
+ $this->assertRaw('<div' . new Attribute(['data-contextual-id' => $id, 'data-contextual-token' => $id_token]) . '></div>', format_string('Contextual link placeholder with id @id exists.', ['@id' => $id]));
+ $this->assertRaw('<div' . new Attribute(['data-contextual-id' => $cached_id, 'data-contextual-token' => $cached_id_token]) . '></div>', format_string('Contextual link placeholder with id @id exists.', ['@id' => $cached_id]));
// Get server-rendered contextual links.
// @see \Drupal\contextual\Tests\ContextualDynamicContextTest:renderContextualLinks()
- $post = ['ids[0]' => $id, 'ids[1]' => $cached_id];
+ $post = ['ids[0]' => $id, 'ids[1]' => $cached_id, 'tokens[0]' => $id_token, 'tokens[1]' => $cached_id_token];
$url = 'contextual/render?_format=json,destination=test-page';
$this->getSession()->getDriver()->getClient()->request('POST', $url, $post);
$this->assertResponse(200);
diff --git a/web/core/modules/content_moderation/src/Plugin/Validation/Constraint/ModerationStateConstraint.php b/web/core/modules/content_moderation/src/Plugin/Validation/Constraint/ModerationStateConstraint.php
index 7f7c756b6b..4fcde36059 100644
--- a/web/core/modules/content_moderation/src/Plugin/Validation/Constraint/ModerationStateConstraint.php
+++ b/web/core/modules/content_moderation/src/Plugin/Validation/Constraint/ModerationStateConstraint.php
@@ -16,5 +16,6 @@ class ModerationStateConstraint extends Constraint {
public $message = 'Invalid state transition from %from to %to';
public $invalidStateMessage = 'State %state does not exist on %workflow workflow';
+ public $invalidTransitionAccess = 'You do not have access to transition from %original_state to %new_state';
}
diff --git a/web/core/modules/content_moderation/src/Plugin/Validation/Constraint/ModerationStateConstraintValidator.php b/web/core/modules/content_moderation/src/Plugin/Validation/Constraint/ModerationStateConstraintValidator.php
index 4b879737da..c3b9c815fe 100644
--- a/web/core/modules/content_moderation/src/Plugin/Validation/Constraint/ModerationStateConstraintValidator.php
+++ b/web/core/modules/content_moderation/src/Plugin/Validation/Constraint/ModerationStateConstraintValidator.php
@@ -2,10 +2,13 @@
namespace Drupal\content_moderation\Plugin\Validation\Constraint;
+use Drupal\content_moderation\StateTransitionValidationInterface;
use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
+use Drupal\Core\Entity\ContentEntityInterface;
use Drupal\Core\Entity\EntityInterface;
use Drupal\Core\Entity\EntityTypeManagerInterface;
use Drupal\content_moderation\ModerationInformationInterface;
+use Drupal\Core\Session\AccountInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\Validator\Constraint;
use Symfony\Component\Validator\ConstraintValidator;
@@ -29,6 +32,20 @@ class ModerationStateConstraintValidator extends ConstraintValidator implements
*/
protected $moderationInformation;
+ /**
+ * The current user.
+ *
+ * @var \Drupal\Core\Session\AccountInterface
+ */
+ protected $currentUser;
+
+ /**
+ * The state transition validation service.
+ *
+ * @var \Drupal\content_moderation\StateTransitionValidationInterface
+ */
+ protected $stateTransitionValidation;
+
/**
* Creates a new ModerationStateConstraintValidator instance.
*
@@ -36,10 +53,16 @@ class ModerationStateConstraintValidator extends ConstraintValidator implements
* The entity type manager.
* @param \Drupal\content_moderation\ModerationInformationInterface $moderation_information
* The moderation information.
+ * @param \Drupal\Core\Session\AccountInterface $current_user
+ * The current user.
+ * @param \Drupal\content_moderation\StateTransitionValidationInterface $state_transition_validation
+ * The state transition validation service.
*/
- public function __construct(EntityTypeManagerInterface $entity_type_manager, ModerationInformationInterface $moderation_information) {
+ public function __construct(EntityTypeManagerInterface $entity_type_manager, ModerationInformationInterface $moderation_information, AccountInterface $current_user, StateTransitionValidationInterface $state_transition_validation) {
$this->entityTypeManager = $entity_type_manager;
$this->moderationInformation = $moderation_information;
+ $this->currentUser = $current_user;
+ $this->stateTransitionValidation = $state_transition_validation;
}
/**
@@ -48,7 +71,9 @@ public function __construct(EntityTypeManagerInterface $entity_type_manager, Mod
public static function create(ContainerInterface $container) {
return new static(
$container->get('entity_type.manager'),
- $container->get('content_moderation.moderation_information')
+ $container->get('content_moderation.moderation_information'),
+ $container->get('current_user'),
+ $container->get('content_moderation.state_transition_validation')
);
}
@@ -76,32 +101,59 @@ public function validate($value, Constraint $constraint) {
return;
}
+ $new_state = $workflow->getTypePlugin()->getState($entity->moderation_state->value);
+ $original_state = $this->getOriginalOrInitialState($entity);
+
// If a new state is being set and there is an existing state, validate
// there is a valid transition between them.
+ if (!$original_state->canTransitionTo($new_state->id())) {
+ $this->context->addViolation($constraint->message, [
+ '%from' => $original_state->label(),
+ '%to' => $new_state->label(),
+ ]);
+ }
+ else {
+ // If we're sure the transition exists, make sure the user has permission
+ // to use it.
+ if (!$this->stateTransitionValidation->isTransitionValid($workflow, $original_state, $new_state, $this->currentUser)) {
+ $this->context->addViolation($constraint->invalidTransitionAccess, [
+ '%original_state' => $original_state->label(),
+ '%new_state' => $new_state->label(),
+ ]);
+ }
+ }
+ }
+
+ /**
+ * Gets the original or initial state of the given entity.
+ *
+ * When a state is being validated, the original state is used to validate
+ * that a valid transition exists for target state and the user has access
+ * to the transition between those two states. If the entity has been
+ * moderated before, we can load the original unmodified revision and
+ * translation for this state.
+ *
+ * If the entity is new we need to load the initial state from the workflow.
+ * Even if a value was assigned to the moderation_state field, the initial
+ * state is used to compute an appropriate transition for the purposes of
+ * validation.
+ *
+ * @return \Drupal\workflows\StateInterface
+ * The original or default moderation state.
+ */
+ protected function getOriginalOrInitialState(ContentEntityInterface $entity) {
+ $state = NULL;
+ $workflow_type = $this->moderationInformation->getWorkflowForEntity($entity)->getTypePlugin();
if (!$entity->isNew() && !$this->isFirstTimeModeration($entity)) {
$original_entity = $this->entityTypeManager->getStorage($entity->getEntityTypeId())->loadRevision($entity->getLoadedRevisionId());
if (!$entity->isDefaultTranslation() && $original_entity->hasTranslation($entity->language()->getId())) {
$original_entity = $original_entity->getTranslation($entity->language()->getId());
}
-
- // If the state of the original entity doesn't exist on the workflow,
- // we cannot do any further validation of transitions, because none will
- // be setup for a state that doesn't exist. Instead allow any state to
- // take its place.
- if (!$workflow->getTypePlugin()->hasState($original_entity->moderation_state->value)) {
- return;
- }
-
- $new_state = $workflow->getTypePlugin()->getState($entity->moderation_state->value);
- $original_state = $workflow->getTypePlugin()->getState($original_entity->moderation_state->value);
-
- if (!$original_state->canTransitionTo($new_state->id())) {
- $this->context->addViolation($constraint->message, [
- '%from' => $original_state->label(),
- '%to' => $new_state->label()
- ]);
+ if ($workflow_type->hasState($original_entity->moderation_state->value)) {
+ $state = $workflow_type->getState($original_entity->moderation_state->value);
}
}
+ return $state ?: $workflow_type->getInitialState($entity);
}
/**
diff --git a/web/core/modules/content_moderation/src/StateTransitionValidation.php b/web/core/modules/content_moderation/src/StateTransitionValidation.php
index 01b2ad8458..35d657e550 100644
--- a/web/core/modules/content_moderation/src/StateTransitionValidation.php
+++ b/web/core/modules/content_moderation/src/StateTransitionValidation.php
@@ -4,7 +4,9 @@
use Drupal\Core\Entity\ContentEntityInterface;
use Drupal\Core\Session\AccountInterface;
+use Drupal\workflows\StateInterface;
use Drupal\workflows\Transition;
+use Drupal\workflows\WorkflowInterface;
/**
* Validates whether a certain state transition is allowed.
@@ -47,4 +49,12 @@ public function getValidTransitions(ContentEntityInterface $entity, AccountInter
});
}
+ /**
+ * {@inheritdoc}
+ */
+ public function isTransitionValid(WorkflowInterface $workflow, StateInterface $original_state, StateInterface $new_state, AccountInterface $user) {
+ $transition = $workflow->getTypePlugin()->getTransitionFromStateToState($original_state->id(), $new_state->id());
+ return $user->hasPermission('use ' . $workflow->id() . ' transition ' . $transition->id());
+ }
+
}
diff --git a/web/core/modules/content_moderation/src/StateTransitionValidationInterface.php b/web/core/modules/content_moderation/src/StateTransitionValidationInterface.php
index 1acbf052fd..c793fe53e2 100644
--- a/web/core/modules/content_moderation/src/StateTransitionValidationInterface.php
+++ b/web/core/modules/content_moderation/src/StateTransitionValidationInterface.php
@@ -4,6 +4,8 @@
use Drupal\Core\Entity\ContentEntityInterface;
use Drupal\Core\Session\AccountInterface;
+use Drupal\workflows\StateInterface;
+use Drupal\workflows\WorkflowInterface;
/**
* Validates whether a certain state transition is allowed.
@@ -23,4 +25,21 @@ interface StateTransitionValidationInterface {
*/
public function getValidTransitions(ContentEntityInterface $entity, AccountInterface $user);
+ /**
+ * Checks if a transition between two states if valid for the given user.
+ *
+ * @param \Drupal\workflows\WorkflowInterface $workflow
+ * The workflow entity.
+ * @param \Drupal\workflows\StateInterface $original_state
+ * The original workflow state.
+ * @param \Drupal\workflows\StateInterface $new_state
+ * The new workflow state.
+ * @param \Drupal\Core\Session\AccountInterface $user
+ * The user to validate.
+ *
+ * @return bool
+ * Returns TRUE if transition is valid, otherwise FALSE.
+ */
+ public function isTransitionValid(WorkflowInterface $workflow, StateInterface $original_state, StateInterface $new_state, AccountInterface $user);
+
}
diff --git a/web/core/modules/content_moderation/tests/src/Functional/ModerationStateNodeTest.php b/web/core/modules/content_moderation/tests/src/Functional/ModerationStateNodeTest.php
index 11deaa72c0..5fd168d0bb 100644
--- a/web/core/modules/content_moderation/tests/src/Functional/ModerationStateNodeTest.php
+++ b/web/core/modules/content_moderation/tests/src/Functional/ModerationStateNodeTest.php
@@ -158,32 +158,15 @@ public function testNoContentModerationPermissions() {
]);
$this->drupalLogin($limited_user);
- // Check the user can add content, but can't see the moderation state
- // select.
+ // Check the user can see the content entity form, but can't see the
+ // moderation state select or save the entity form.
$this->drupalGet('node/add/moderated_content');
$session_assert->statusCodeEquals(200);
$session_assert->fieldNotExists('moderation_state[0][state]');
$this->drupalPostForm(NULL, [
'title[0][value]' => 'moderated content',
], 'Save');
-
- // Manually move the content to archived because the user doesn't have
- // permission to do this.
- $node = $this->getNodeByTitle('moderated content');
- $node->moderation_state->value = 'archived';
- $node->save();
-
- // Check the user can see the current state but not the select.
- $this->drupalGet('node/' . $node->id() . '/edit');
- $session_assert->statusCodeEquals(200);
- $session_assert->pageTextContains('Archived');
- $session_assert->fieldNotExists('moderation_state[0][state]');
- $this->drupalPostForm(NULL, [], 'Save');
-
- // When saving they should still be on the edit form, and see the validation
- // error message.
- $session_assert->pageTextContains('Edit Moderated content moderated content');
- $session_assert->pageTextContains('Invalid state transition from Archived to Archived');
+ $session_assert->pageTextContains('You do not have access to transition from Draft to Draft');
}
}
diff --git a/web/core/modules/content_moderation/tests/src/Kernel/EntityStateChangeValidationTest.php b/web/core/modules/content_moderation/tests/src/Kernel/EntityStateChangeValidationTest.php
index aeed8486f5..9f304dd3f9 100644
--- a/web/core/modules/content_moderation/tests/src/Kernel/EntityStateChangeValidationTest.php
+++ b/web/core/modules/content_moderation/tests/src/Kernel/EntityStateChangeValidationTest.php
@@ -6,6 +6,7 @@
use Drupal\language\Entity\ConfigurableLanguage;
use Drupal\node\Entity\Node;
use Drupal\node\Entity\NodeType;
+use Drupal\Tests\user\Traits\UserCreationTrait;
use Drupal\workflows\Entity\Workflow;
/**
@@ -14,6 +15,8 @@
*/
class EntityStateChangeValidationTest extends KernelTestBase {
+ use UserCreationTrait;
+
/**
* {@inheritdoc}
*/
@@ -27,6 +30,13 @@ class EntityStateChangeValidationTest extends KernelTestBase {
'workflows',
];
+ /**
+ * An admin user.
+ *
+ * @var \Drupal\Core\Session\AccountInterface
+ */
+ protected $adminUser;
+
/**
* {@inheritdoc}
*/
@@ -38,6 +48,9 @@ protected function setUp() {
$this->installEntitySchema('user');
$this->installEntitySchema('content_moderation_state');
$this->installConfig('content_moderation');
+ $this->installSchema('system', ['sequences']);
+
+ $this->adminUser = $this->createUser(array_keys($this->container->get('user.permissions')->getPermissions()));
}
/**
@@ -46,6 +59,8 @@ protected function setUp() {
* @covers ::validate
*/
public function testValidTransition() {
+ $this->setCurrentUser($this->adminUser);
+
$node_type = NodeType::create([
'type' => 'example',
]);
@@ -74,6 +89,8 @@ public function testValidTransition() {
* @covers ::validate
*/
public function testInvalidTransition() {
+ $this->setCurrentUser($this->adminUser);
+
$node_type = NodeType::create([
'type' => 'example',
]);
@@ -123,6 +140,7 @@ public function testInvalidState() {
* Test validation with content that has no initial state or an invalid state.
*/
public function testInvalidStateWithoutExisting() {
+ $this->setCurrentUser($this->adminUser);
// Create content without moderation enabled for the content type.
$node_type = NodeType::create([
'type' => 'example',
@@ -154,15 +172,24 @@ public function testInvalidStateWithoutExisting() {
// validating.
$workflow->getTypePlugin()->deleteState('deleted_state');
$workflow->save();
+
+ // When there is an invalid state, the content will revert to "draft". This
+ // will allow a draft to draft transition.
$node->moderation_state->value = 'draft';
$violations = $node->validate();
$this->assertCount(0, $violations);
+ // This will disallow a draft to archived transition.
+ $node->moderation_state->value = 'archived';
+ $violations = $node->validate();
+ $this->assertCount(1, $violations);
}
/**
* Test state transition validation with multiple languages.
*/
public function testInvalidStateMultilingual() {
+ $this->setCurrentUser($this->adminUser);
+
ConfigurableLanguage::createFromLangcode('fr')->save();
$node_type = NodeType::create([
'type' => 'example',
@@ -218,6 +245,8 @@ public function testInvalidStateMultilingual() {
* Tests that content without prior moderation information can be moderated.
*/
public function testExistingContentWithNoModeration() {
+ $this->setCurrentUser($this->adminUser);
+
$node_type = NodeType::create([
'type' => 'example',
]);
@@ -252,6 +281,8 @@ public function testExistingContentWithNoModeration() {
* Tests that content without prior moderation information can be translated.
*/
public function testExistingMultilingualContentWithNoModeration() {
+ $this->setCurrentUser($this->adminUser);
+
// Enable French.
ConfigurableLanguage::createFromLangcode('fr')->save();
@@ -291,4 +322,81 @@ public function testExistingMultilingualContentWithNoModeration() {
$node_fr->save();
}
+ /**
+ * @dataProvider transitionAccessValidationTestCases
+ */
+ public function testTransitionAccessValidation($permissions, $target_state, $messages) {
+ $node_type = NodeType::create([
+ 'type' => 'example',
+ ]);
+ $node_type->save();
+ $workflow = Workflow::load('editorial');
+ $workflow->getTypePlugin()->addState('foo', 'Foo');
+ $workflow->getTypePlugin()->addTransition('draft_to_foo', 'Draft to foo', ['draft'], 'foo');
+ $workflow->getTypePlugin()->addTransition('foo_to_foo', 'Foo to foo', ['foo'], 'foo');
+ $workflow->getTypePlugin()->addEntityTypeAndBundle('node', 'example');
+ $workflow->save();
+
+ $this->setCurrentUser($this->createUser($permissions));
+
+ $node = Node::create([
+ 'type' => 'example',
+ 'title' => 'Test content',
+ 'moderation_state' => $target_state,
+ ]);
+ $this->assertTrue($node->isNew());
+ $violations = $node->validate();
+ $this->assertCount(count($messages), $violations);
+ foreach ($messages as $i => $message) {
+ $this->assertEquals($message, $violations->get($i)->getMessage());
+ }
+ }
+
+ /**
+ * Test cases for ::testTransitionAccessValidation.
+ */
+ public function transitionAccessValidationTestCases() {
+ return [
+ 'Invalid transition, no permissions validated' => [
+ [],
+ 'archived',
+ ['Invalid state transition from <em class="placeholder">Draft</em> to <em class="placeholder">Archived</em>'],
+ ],
+ 'Valid transition, missing permission' => [
+ [],
+ 'published',
+ ['You do not have access to transition from <em class="placeholder">Draft</em> to <em class="placeholder">Published</em>'],
+ ],
+ 'Valid transition, granted published permission' => [
+ ['use editorial transition publish'],
+ 'published',
+ [],
+ ],
+ 'Valid transition, granted draft permission' => [
+ ['use editorial transition create_new_draft'],
+ 'draft',
+ [],
+ ],
+ 'Valid transition, incorrect permission granted' => [
+ ['use editorial transition create_new_draft'],
+ 'published',
+ ['You do not have access to transition from <em class="placeholder">Draft</em> to <em class="placeholder">Published</em>'],
+ ],
+ // Test with an additional state and set of transitions, since the
+ // "published" transition can start from either "draft" or "published", it
+ // does not capture bugs that fail to correctly distinguish the initial
+ // workflow state from the set state of a new entity.
+ 'Valid transition, granted foo permission' => [
+ ['use editorial transition draft_to_foo'],
+ 'foo',
+ [],
+ ],
+ 'Valid transition, incorrect foo permission granted' => [
+ ['use editorial transition foo_to_foo'],
+ 'foo',
+ ['You do not have access to transition from <em class="placeholder">Draft</em> to <em class="placeholder">Foo</em>'],
+ ],
+ ];
+ }
+
}
diff --git a/web/core/modules/contextual/contextual.module b/web/core/modules/contextual/contextual.module
index a814c391f1..901ecf116f 100644
--- a/web/core/modules/contextual/contextual.module
+++ b/web/core/modules/contextual/contextual.module
@@ -191,13 +191,19 @@ function _contextual_links_to_id($contextual_links) {
/**
* Unserializes the result of _contextual_links_to_id().
*
- * @see _contextual_links_to_id
+ * Note that $id is user input. Before calling this method the ID should be
+ * checked against the token stored in the 'data-contextual-token' attribute
+ * which is passed via the 'tokens' request parameter to
+ * \Drupal\contextual\ContextualController::render().
*
* @param string $id
* A serialized representation of a #contextual_links property value array.
*
* @return array
* The value for a #contextual_links property.
+ *
+ * @see _contextual_links_to_id()
+ * @see \Drupal\contextual\ContextualController::render()
*/
function _contextual_id_to_links($id) {
$contextual_links = [];
diff --git a/web/core/modules/contextual/contextual.post_update.php b/web/core/modules/contextual/contextual.post_update.php
new file mode 100644
index 0000000000..8decad05f0
--- /dev/null
+++ b/web/core/modules/contextual/contextual.post_update.php
@@ -0,0 +1,14 @@
+<?php
+
+/**
+ * @file
+ * Post update functions for Contextual Links.
+ */
+
+/**
+ * Ensure new page loads use the updated JS and get the updated markup.
+ */
+function contextual_post_update_fixed_endpoint_and_markup() {
+ // Empty update to trigger a change to css_js_query_string and invalidate
+ // cached markup.
+}
diff --git a/web/core/modules/contextual/js/contextual.es6.js b/web/core/modules/contextual/js/contextual.es6.js
index 701ed126cb..c864006364 100644
--- a/web/core/modules/contextual/js/contextual.es6.js
+++ b/web/core/modules/contextual/js/contextual.es6.js
@@ -154,12 +154,17 @@
// Collect the IDs for all contextual links placeholders.
const ids = [];
$placeholders.each(function () {
- ids.push($(this).attr('data-contextual-id'));
+ ids.push({
+ id: $(this).attr('data-contextual-id'),
+ token: $(this).attr('data-contextual-token'),
+ });
});
+ const uncachedIDs = [];
+ const uncachedTokens = [];
// Update all contextual links placeholders whose HTML is cached.
- const uncachedIDs = _.filter(ids, (contextualID) => {
- const html = storage.getItem(`Drupal.contextual.${contextualID}`);
+ ids.forEach((contextualID) => {
+ const html = storage.getItem(`Drupal.contextual.${contextualID.id}`);
if (html && html.length) {
// Initialize after the current execution cycle, to make the AJAX
// request for retrieving the uncached contextual links as soon as
@@ -167,11 +172,12 @@
// the chance to set up an event listener on the Backbone collection
// Drupal.contextual.collection.
window.setTimeout(() => {
- initContextual($context.find(`[data-contextual-id="${contextualID}"]`), html);
+ initContextual($context.find(`[data-contextual-id="${contextualID.id}"]`), html);
});
- return false;
+ return;
}
- return true;
+ uncachedIDs.push(contextualID.id);
+ uncachedTokens.push(contextualID.token);
});
// Perform an AJAX request to let the server render the contextual links
@@ -180,7 +186,7 @@
$.ajax({
url: Drupal.url('contextual/render'),
type: 'POST',
- data: { 'ids[]': uncachedIDs },
+ data: { 'ids[]': uncachedIDs, 'tokens[]': uncachedTokens },
dataType: 'json',
success(results) {
_.each(results, (html, contextualID) => {
diff --git a/web/core/modules/contextual/js/contextual.js b/web/core/modules/contextual/js/contextual.js
index ed210d070d..e0eaddf899 100644
--- a/web/core/modules/contextual/js/contextual.js
+++ b/web/core/modules/contextual/js/contextual.js
@@ -95,25 +95,32 @@
var ids = [];
$placeholders.each(function () {
- ids.push($(this).attr('data-contextual-id'));
+ ids.push({
+ id: $(this).attr('data-contextual-id'),
+ token: $(this).attr('data-contextual-token')
+ });
});
- var uncachedIDs = _.filter(ids, function (contextualID) {
- var html = storage.getItem('Drupal.contextual.' + contextualID);
+ var uncachedIDs = [];
+ var uncachedTokens = [];
+
+ ids.forEach(function (contextualID) {
+ var html = storage.getItem('Drupal.contextual.' + contextualID.id);
if (html && html.length) {
window.setTimeout(function () {
- initContextual($context.find('[data-contextual-id="' + contextualID + '"]'), html);
+ initContextual($context.find('[data-contextual-id="' + contextualID.id + '"]'), html);
});
- return false;
+ return;
}
- return true;
+ uncachedIDs.push(contextualID.id);
+ uncachedTokens.push(contextualID.token);
});
if (uncachedIDs.length > 0) {
$.ajax({
url: Drupal.url('contextual/render'),
type: 'POST',
- data: { 'ids[]': uncachedIDs },
+ data: { 'ids[]': uncachedIDs, 'tokens[]': uncachedTokens },
dataType: 'json',
success: function success(results) {
_.each(results, function (html, contextualID) {
diff --git a/web/core/modules/contextual/src/ContextualController.php b/web/core/modules/contextual/src/ContextualController.php
index 59a2ba0f95..206fb307e5 100644
--- a/web/core/modules/contextual/src/ContextualController.php
+++ b/web/core/modules/contextual/src/ContextualController.php
@@ -2,8 +2,10 @@
namespace Drupal\contextual;
+use Drupal\Component\Utility\Crypt;
use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
use Drupal\Core\Render\RendererInterface;
+use Drupal\Core\Site\Settings;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\HttpFoundation\JsonResponse;
use Symfony\Component\HttpFoundation\Request;
@@ -56,8 +58,16 @@ public function render(Request $request) {
throw new BadRequestHttpException(t('No contextual ids specified.'));
}
+ $tokens = $request->request->get('tokens');
+ if (!isset($tokens)) {
+ throw new BadRequestHttpException(t('No contextual ID tokens specified.'));
+ }
+
$rendered = [];
- foreach ($ids as $id) {
+ foreach ($ids as $key => $id) {
+ if (!isset($tokens[$key]) || !Crypt::hashEquals($tokens[$key], Crypt::hmacBase64($id, Settings::getHashSalt() . \Drupal::service('private_key')->get()))) {
+ throw new BadRequestHttpException('Invalid contextual ID specified.');
+ }
$element = [
'#type' => 'contextual_links',
'#contextual_links' => _contextual_id_to_links($id),
diff --git a/web/core/modules/contextual/src/Element/ContextualLinksPlaceholder.php b/web/core/modules/contextual/src/Element/ContextualLinksPlaceholder.php
index 2fba8db3f1..86d485692d 100644
--- a/web/core/modules/contextual/src/Element/ContextualLinksPlaceholder.php
+++ b/web/core/modules/contextual/src/Element/ContextualLinksPlaceholder.php
@@ -2,9 +2,11 @@
namespace Drupal\contextual\Element;
+use Drupal\Component\Render\FormattableMarkup;
+use Drupal\Component\Utility\Crypt;
+use Drupal\Core\Site\Settings;
use Drupal\Core\Template\Attribute;
use Drupal\Core\Render\Element\RenderElement;
-use Drupal\Component\Utility\SafeMarkup;
/**
* Provides a contextual_links_placeholder element.
@@ -43,7 +45,12 @@ public function getInfo() {
* @see _contextual_links_to_id()
*/
public static function preRenderPlaceholder(array $element) {
- $element['#markup'] = SafeMarkup::format('<div@attributes></div>', ['@attributes' => new Attribute(['data-contextual-id' => $element['#id']])]);
+ $token = Crypt::hmacBase64($element['#id'], Settings::getHashSalt() . \Drupal::service('private_key')->get());
+ $attribute = new Attribute([
+ 'data-contextual-id' => $element['#id'],
+ 'data-contextual-token' => $token,
+ ]);
+ $element['#markup'] = new FormattableMarkup('<div@attributes></div>', ['@attributes' => $attribute]);
return $element;
}
diff --git a/web/core/modules/contextual/src/Tests/ContextualDynamicContextTest.php b/web/core/modules/contextual/src/Tests/ContextualDynamicContextTest.php
index 00ade5a5ac..73a7428ef1 100644
--- a/web/core/modules/contextual/src/Tests/ContextualDynamicContextTest.php
+++ b/web/core/modules/contextual/src/Tests/ContextualDynamicContextTest.php
@@ -3,6 +3,8 @@
namespace Drupal\contextual\Tests;
use Drupal\Component\Serialization\Json;
+use Drupal\Component\Utility\Crypt;
+use Drupal\Core\Site\Settings;
use Drupal\Core\Url;
use Drupal\language\Entity\ConfigurableLanguage;
use Drupal\simpletest\WebTestBase;
@@ -140,6 +142,43 @@ public function testDifferentPermissions() {
$this->assertRaw('<li class="menu-testcontextual-hidden-manage-edit"><a href="' . base_path() . 'menu-test-contextual/1/edit" class="use-ajax" data-dialog-type="modal" data-is-something>Edit menu - contextual</a></li>');
}
+ /**
+ * Tests the contextual placeholder content is protected by a token.
+ */
+ public function testTokenProtection() {
+ $this->drupalLogin($this->editorUser);
+
+ // Create a node that will have a contextual link.
+ $node1 = $this->drupalCreateNode(['type' => 'article', 'promote' => 1]);
+
+ // Now, on the front page, all article nodes should have contextual links
+ // placeholders, as should the view that contains them.
+ $id = 'node:node=' . $node1->id() . ':changed=' . $node1->getChangedTime() . '&langcode=en';
+
+ // Editor user: can access contextual links and can edit articles.
+ $this->drupalGet('node');
+ $this->assertContextualLinkPlaceHolder($id);
+
+ $post = ['ids[0]' => $id];
+ $this->drupalPostWithFormat('contextual/render', 'json', $post, ['query' => ['destination' => 'node']]);
+ $this->assertResponse(400);
+ $this->assertRaw('No contextual ID tokens specified.');
+
+ $post = ['ids[0]' => $id, 'tokens[0]' => 'wrong_token'];
+ $this->drupalPostWithFormat('contextual/render', 'json', $post, ['query' => ['destination' => 'node']]);
+ $this->assertResponse(400);
+ $this->assertRaw('Invalid contextual ID specified.');
+
+ $post = ['ids[0]' => $id, 'tokens[wrong_key]' => $this->createContextualIdToken($id)];
+ $this->drupalPostWithFormat('contextual/render', 'json', $post, ['query' => ['destination' => 'node']]);
+ $this->assertResponse(400);
+ $this->assertRaw('Invalid contextual ID specified.');
+
+ $post = ['ids[0]' => $id, 'tokens[0]' => $this->createContextualIdToken($id)];
+ $this->drupalPostWithFormat('contextual/render', 'json', $post, ['query' => ['destination' => 'node']]);
+ $this->assertResponse(200);
+ }
+
/**
* Asserts that a contextual link placeholder with the given id exists.
*
@@ -150,7 +189,11 @@ public function testDifferentPermissions() {
* The result of the assertion.
*/
protected function assertContextualLinkPlaceHolder($id) {
- return $this->assertRaw('<div' . new Attribute(['data-contextual-id' => $id]) . '></div>', format_string('Contextual link placeholder with id @id exists.', ['@id' => $id]));
+ $attribute = new Attribute([
+ 'data-contextual-id' => $id,
+ 'data-contextual-token' => $this->createContextualIdToken($id),
+ ]);
+ return $this->assertRaw('<div' . $attribute . '></div>', format_string('Contextual link placeholder with id @id exists.', ['@id' => $id]));
}
/**
@@ -163,7 +206,11 @@ protected function assertContextualLinkPlaceHolder($id) {
* The result of the assertion.
*/
protected function assertNoContextualLinkPlaceHolder($id) {
- return $this->assertNoRaw('<div' . new Attribute(['data-contextual-id' => $id]) . '></div>', format_string('Contextual link placeholder with id @id does not exist.', ['@id' => $id]));
+ $attribute = new Attribute([
+ 'data-contextual-id' => $id,
+ 'data-contextual-token' => $this->createContextualIdToken($id),
+ ]);
+ return $this->assertNoRaw('<div' . $attribute . '></div>', format_string('Contextual link placeholder with id @id does not exist.', ['@id' => $id]));
}
/**
@@ -181,8 +228,22 @@ protected function renderContextualLinks($ids, $current_path) {
$post = [];
for ($i = 0; $i < count($ids); $i++) {
$post['ids[' . $i . ']'] = $ids[$i];
+ $post['tokens[' . $i . ']'] = $this->createContextualIdToken($ids[$i]);
}
return $this->drupalPostWithFormat('contextual/render', 'json', $post, ['query' => ['destination' => $current_path]]);
}
+ /**
+ * Creates a contextual ID token.
+ *
+ * @param string $id
+ * The contextual ID to create a token for.
+ *
+ * @return string
+ * The contextual ID token.
+ */
+ protected function createContextualIdToken($id) {
+ return Crypt::hmacBase64($id, Settings::getHashSalt() . $this->container->get('private_key')->get());
+ }
+
}
diff --git a/web/core/modules/node/src/Tests/NodeRevisionsTest.php b/web/core/modules/node/src/Tests/NodeRevisionsTest.php
index c36452d9ab..5b85688a13 100644
--- a/web/core/modules/node/src/Tests/NodeRevisionsTest.php
+++ b/web/core/modules/node/src/Tests/NodeRevisionsTest.php
@@ -2,6 +2,8 @@
namespace Drupal\node\Tests;
+use Drupal\Component\Utility\Crypt;
+use Drupal\Core\Site\Settings;
use Drupal\Core\Url;
use Drupal\field\Entity\FieldConfig;
use Drupal\field\Entity\FieldStorageConfig;
@@ -367,6 +369,10 @@ protected function renderContextualLinks(array $ids, $current_path) {
$post = [];
for ($i = 0; $i < count($ids); $i++) {
$post['ids[' . $i . ']'] = $ids[$i];
+ $post['tokens[' . $i . ']'] = Crypt::hmacBase64(
+ $ids[$i],
+ Settings::getHashSalt() . \Drupal::service('private_key')->get()
+ );
}
$response = $this->drupalPost('contextual/render', 'application/json', $post, ['query' => ['destination' => $current_path]]);
diff --git a/web/core/modules/node/src/Tests/Views/NodeContextualLinksTest.php b/web/core/modules/node/src/Tests/Views/NodeContextualLinksTest.php
index dc23d0ce55..45b4d086b4 100644
--- a/web/core/modules/node/src/Tests/Views/NodeContextualLinksTest.php
+++ b/web/core/modules/node/src/Tests/Views/NodeContextualLinksTest.php
@@ -3,6 +3,8 @@
namespace Drupal\node\Tests\Views;
use Drupal\Component\Serialization\Json;
+use Drupal\Component\Utility\Crypt;
+use Drupal\Core\Site\Settings;
use Drupal\user\Entity\User;
/**
@@ -66,6 +68,10 @@ protected function renderContextualLinks($ids, $current_path) {
$post = [];
for ($i = 0; $i < count($ids); $i++) {
$post['ids[' . $i . ']'] = $ids[$i];
+ $post['tokens[' . $i . ']'] = Crypt::hmacBase64(
+ $ids[$i],
+ Settings::getHashSalt() . \Drupal::service('private_key')->get()
+ );
}
// Serialize POST values.
diff --git a/web/core/modules/path/tests/src/Functional/PathAliasTest.php b/web/core/modules/path/tests/src/Functional/PathAliasTest.php
index a12b282d67..7ac5ca1cf4 100644
--- a/web/core/modules/path/tests/src/Functional/PathAliasTest.php
+++ b/web/core/modules/path/tests/src/Functional/PathAliasTest.php
@@ -5,6 +5,7 @@
use Drupal\Component\Utility\Unicode;
use Drupal\Core\Cache\Cache;
use Drupal\Core\Database\Database;
+use Drupal\Core\Url;
/**
* Add, edit, delete, and change alias and verify its consistency in the
@@ -25,7 +26,7 @@ protected function setUp() {
parent::setUp();
// Create test user and log in.
- $web_user = $this->drupalCreateUser(['create page content', 'edit own page content', 'administer url aliases', 'create url aliases']);
+ $web_user = $this->drupalCreateUser(['create page content', 'edit own page content', 'administer url aliases', 'create url aliases', 'access content overview']);
$this->drupalLogin($web_user);
}
@@ -328,6 +329,34 @@ public function testNodeAlias() {
$node5->delete();
$path_alias = \Drupal::service('path.alias_storage')->lookupPathAlias('/node/' . $node5->id(), $node5->language()->getId());
$this->assertFalse($path_alias, 'Alias was successfully deleted when the referenced node was deleted.');
+
+ // Create sixth test node.
+ $node6 = $this->drupalCreateNode();
+
+ // Create an invalid alias with two leading slashes and verify that the
+ // extra slash is removed when the link is generated. This ensures that URL
+ // aliases cannot be used to inject external URLs.
+ // @todo The user interface should either display an error message or
+ // automatically trim these invalid aliases, rather than allowing them to
+ // be silently created, at which point the functional aspects of this
+ // test will need to be moved elsewhere and switch to using a
+ // programmatically-created alias instead.
+ $alias = $this->randomMachineName(8);
+ $edit = ['path[0][alias]' => '//' . $alias];
+ $this->drupalPostForm($node6->toUrl('edit-form'), $edit, t('Save'));
+ $this->drupalGet(Url::fromRoute('system.admin_content'));
+ // This checks the link href before clicking it, rather than using
+ // \Drupal\Tests\BrowserTestBase::assertSession()->addressEquals() after
+ // clicking it, because the test browser does not always preserve the
+ // correct number of slashes in the URL when it visits internal links;
+ // using \Drupal\Tests\BrowserTestBase::assertSession()->addressEquals()
+ // would actually make the test pass unconditionally on the testbot (or
+ // anywhere else where Drupal is installed in a subdirectory).
+ $link_xpath = $this->xpath('//a[normalize-space(text())=:label]', [':label' => $node6->getTitle()]);
+ $link_href = $link_xpath[0]->getAttribute('href');
+ $this->assertEquals($link_href, base_path() . $alias);
+ $this->clickLink($node6->getTitle());
+ $this->assertResponse(404);
}
/**
diff --git a/web/core/modules/system/src/Tests/Routing/RouterTest.php b/web/core/modules/system/src/Tests/Routing/RouterTest.php
index 83a9c55b39..8d7c43e86a 100644
--- a/web/core/modules/system/src/Tests/Routing/RouterTest.php
+++ b/web/core/modules/system/src/Tests/Routing/RouterTest.php
@@ -320,6 +320,13 @@ public function testLeadingSlashes() {
$this->drupalGet($url);
$this->assertEqual(1, $this->redirectCount, $url . " redirected to " . $this->url);
$this->assertUrl($request->getUriForPath('/router_test/test1') . '?qs=test');
+
+ // Ensure that external URLs in destination query params are not redirected
+ // to.
+ $url = $request->getUriForPath('/////////////////////////////////////////////////router_test/test1') . '?qs=test&destination=http://www.example.com%5c@drupal8alt.test';
+ $this->drupalGet($url);
+ $this->assertEqual(1, $this->redirectCount, $url . " redirected to " . $this->url);
+ $this->assertUrl($request->getUriForPath('/router_test/test1') . '?qs=test');
}
}
diff --git a/web/core/tests/Drupal/Tests/Component/Utility/UrlHelperTest.php b/web/core/tests/Drupal/Tests/Component/Utility/UrlHelperTest.php
index d185219c9a..beaa472c26 100644
--- a/web/core/tests/Drupal/Tests/Component/Utility/UrlHelperTest.php
+++ b/web/core/tests/Drupal/Tests/Component/Utility/UrlHelperTest.php
@@ -563,6 +563,10 @@ public function providerTestExternalIsLocal() {
['http://example.com/foo', 'http://example.com/bar', FALSE],
['http://example.com', 'http://example.com/bar', FALSE],
['http://example.com/bar', 'http://example.com/bar/', FALSE],
+ // Ensure \ is normalised to / since some browsers do that.
+ ['http://www.example.ca\@example.com', 'http://example.com', FALSE],
+ // Some browsers ignore or strip leading control characters.
+ ["\x00//www.example.ca", 'http://example.com', FALSE],
];
}
diff --git a/web/core/tests/Drupal/Tests/Core/EventSubscriber/RedirectResponseSubscriberTest.php b/web/core/tests/Drupal/Tests/Core/EventSubscriber/RedirectResponseSubscriberTest.php
index 8659a6f126..85b3da313c 100644
--- a/web/core/tests/Drupal/Tests/Core/EventSubscriber/RedirectResponseSubscriberTest.php
+++ b/web/core/tests/Drupal/Tests/Core/EventSubscriber/RedirectResponseSubscriberTest.php
@@ -11,7 +11,6 @@
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
-use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\HttpKernel\KernelEvents;
@@ -192,74 +191,4 @@ public function providerTestDestinationRedirectWithInvalidUrl() {
return $data;
}
- /**
- * Tests that $_GET only contain internal URLs.
- *
- * @covers ::sanitizeDestination
- *
- * @dataProvider providerTestSanitizeDestination
- *
- * @see \Drupal\Component\Utility\UrlHelper::isExternal
- */
- public function testSanitizeDestinationForGet($input, $output) {
- $request = new Request();
- $request->query->set('destination', $input);
-
- $listener = new RedirectResponseSubscriber($this->urlAssembler, $this->requestContext);
- $kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
- $event = new GetResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST);
-
- $dispatcher = new EventDispatcher();
- $dispatcher->addListener(KernelEvents::REQUEST, [$listener, 'sanitizeDestination'], 100);
- $dispatcher->dispatch(KernelEvents::REQUEST, $event);
-
- $this->assertEquals($output, $request->query->get('destination'));
- }
-
- /**
- * Tests that $_REQUEST['destination'] only contain internal URLs.
- *
- * @covers ::sanitizeDestination
- *
- * @dataProvider providerTestSanitizeDestination
- *
- * @see \Drupal\Component\Utility\UrlHelper::isExternal
- */
- public function testSanitizeDestinationForPost($input, $output) {
- $request = new Request();
- $request->request->set('destination', $input);
-
- $listener = new RedirectResponseSubscriber($this->urlAssembler, $this->requestContext);
- $kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
- $event = new GetResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST);
-
- $dispatcher = new EventDispatcher();
- $dispatcher->addListener(KernelEvents::REQUEST, [$listener, 'sanitizeDestination'], 100);
- $dispatcher->dispatch(KernelEvents::REQUEST, $event);
-
- $this->assertEquals($output, $request->request->get('destination'));
- }
-
- /**
- * Data provider for testSanitizeDestination().
- */
- public function providerTestSanitizeDestination() {
- $data = [];
- // Standard internal example node path is present in the 'destination'
- // parameter.
- $data[] = ['node', 'node'];
- // Internal path with one leading slash is allowed.
- $data[] = ['/example.com', '/example.com'];
- // External URL without scheme is not allowed.
- $data[] = ['//example.com/test', ''];
- // Internal URL using a colon is allowed.
- $data[] = ['example:test', 'example:test'];
- // External URL is not allowed.
- $data[] = ['http://example.com', ''];
- // Javascript URL is allowed because it is treated as an internal URL.
- $data[] = ['javascript:alert(0)', 'javascript:alert(0)'];
-
- return $data;
- }
-
}
diff --git a/web/core/tests/Drupal/Tests/Core/Mail/MailManagerTest.php b/web/core/tests/Drupal/Tests/Core/Mail/MailManagerTest.php
index 9ba2fb52b7..7df7dd48b4 100644
--- a/web/core/tests/Drupal/Tests/Core/Mail/MailManagerTest.php
+++ b/web/core/tests/Drupal/Tests/Core/Mail/MailManagerTest.php
@@ -7,6 +7,7 @@
namespace Drupal\Tests\Core\Mail;
+use Drupal\Core\DependencyInjection\ContainerBuilder;
use Drupal\Core\Render\RenderContext;
use Drupal\Core\Render\RendererInterface;
use Drupal\Tests\UnitTestCase;
@@ -103,6 +104,9 @@ protected function setUpMailManager($interface = []) {
'system.mail' => [
'interface' => $interface,
],
+ 'system.site' => [
+ 'mail' => 'test@example.com',
+ ],
]);
$logger_factory = $this->getMock('\Drupal\Core\Logger\LoggerChannelFactoryInterface');
$string_translation = $this->getStringTranslationStub();
@@ -110,6 +114,11 @@ protected function setUpMailManager($interface = []) {
// Construct the manager object and override its discovery.
$this->mailManager = new TestMailManager(new \ArrayObject(), $this->cache, $this->moduleHandler, $this->configFactory, $logger_factory, $string_translation, $this->renderer);
$this->mailManager->setDiscovery($this->discovery);
+
+ // @see \Drupal\Core\Plugin\Factory\ContainerFactory::createInstance()
+ $container = new ContainerBuilder();
+ $container->set('config.factory', $this->configFactory);
+ \Drupal::setContainer($container);
}
/**
diff --git a/web/core/tests/Drupal/Tests/Core/Security/RequestSanitizerTest.php b/web/core/tests/Drupal/Tests/Core/Security/RequestSanitizerTest.php
new file mode 100644
index 0000000000..9a502c13b3
--- /dev/null
+++ b/web/core/tests/Drupal/Tests/Core/Security/RequestSanitizerTest.php
@@ -0,0 +1,371 @@
+<?php
+
+namespace Drupal\Tests\Core\Security;
+
+use Drupal\Core\Security\RequestSanitizer;
+use Drupal\Tests\UnitTestCase;
+use Symfony\Component\HttpFoundation\Request;
+
+/**
+ * Tests RequestSanitizer class.
+ *
+ * @coversDefaultClass \Drupal\Core\Security\RequestSanitizer
+ * @runTestsInSeparateProcesses
+ * @preserveGlobalState disabled
+ * @group Security
+ */
+class RequestSanitizerTest extends UnitTestCase {
+
+ /**
+ * Log of errors triggered during sanitization.
+ *
+ * @var array
+ */
+ protected $errors;
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function setUp() {
+ parent::setUp();
+ $this->errors = [];
+ set_error_handler([$this, "errorHandler"]);
+ }
+
+ /**
+ * Tests RequestSanitizer class.
+ *
+ * @param \Symfony\Component\HttpFoundation\Request $request
+ * The request to sanitize.
+ * @param array $expected
+ * An array of expected request parameters after sanitization. The possible
+ * keys are 'cookies', 'query', 'request' which correspond to the parameter
+ * bags names on the request object. These values are also used to test the
+ * PHP globals post sanitization.
+ * @param array|null $expected_errors
+ * An array of expected errors. If set to NULL then error logging is
+ * disabled.
+ * @param array $whitelist
+ * An array of keys to whitelist and not sanitize.
+ *
+ * @dataProvider providerTestRequestSanitization
+ */
+ public function testRequestSanitization(Request $request, array $expected = [], array $expected_errors = NULL, array $whitelist = []) {
+ // Set up globals.
+ $_GET = $request->query->all();
+ $_POST = $request->request->all();
+ $_COOKIE = $request->cookies->all();
+ $_REQUEST = array_merge($request->query->all(), $request->request->all());
+ $request->server->set('QUERY_STRING', http_build_query($request->query->all()));
+ $_SERVER['QUERY_STRING'] = $request->server->get('QUERY_STRING');
+
+ $request = RequestSanitizer::sanitize($request, $whitelist, is_null($expected_errors) ? FALSE : TRUE);
+
+ // Normalise the expected data.
+ $expected += ['cookies' => [], 'query' => [], 'request' => []];
+ $expected_query_string = http_build_query($expected['query']);
+
+ // Test the request.
+ $this->assertEquals($expected['cookies'], $request->cookies->all());
+ $this->assertEquals($expected['query'], $request->query->all());
+ $this->assertEquals($expected['request'], $request->request->all());
+ $this->assertTrue($request->attributes->get(RequestSanitizer::SANITIZED));
+ // The request object normalizes the request query string.
+ $this->assertEquals(Request::normalizeQueryString($expected_query_string), $request->getQueryString());
+
+ // Test PHP globals.
+ $this->assertEquals($expected['cookies'], $_COOKIE);
+ $this->assertEquals($expected['query'], $_GET);
+ $this->assertEquals($expected['request'], $_POST);
+ $expected_request = array_merge($expected['query'], $expected['request']);
+ $this->assertEquals($expected_request, $_REQUEST);
+ $this->assertEquals($expected_query_string, $_SERVER['QUERY_STRING']);
+
+ // Ensure any expected errors have been triggered.
+ if (!empty($expected_errors)) {
+ foreach ($expected_errors as $expected_error) {
+ $this->assertError($expected_error, E_USER_NOTICE);
+ }
+ }
+ else {
+ $this->assertEquals([], $this->errors);
+ }
+ }
+
+ /**
+ * Data provider for testRequestSanitization.
+ *
+ * @return array
+ */
+ public function providerTestRequestSanitization() {
+ $tests = [];
+
+ $request = new Request(['q' => 'index.php']);
+ $tests['no sanitization GET'] = [$request, ['query' => ['q' => 'index.php']]];
+
+ $request = new Request([], ['field' => 'value']);
+ $tests['no sanitization POST'] = [$request, ['request' => ['field' => 'value']]];
+
+ $request = new Request([], [], [], ['key' => 'value']);
+ $tests['no sanitization COOKIE'] = [$request, ['cookies' => ['key' => 'value']]];
+
+ $request = new Request(['q' => 'index.php'], ['field' => 'value'], [], ['key' => 'value']);
+ $tests['no sanitization GET, POST, COOKIE'] = [$request, ['query' => ['q' => 'index.php'], 'request' => ['field' => 'value'], 'cookies' => ['key' => 'value']]];
+
+ $request = new Request(['q' => 'index.php']);
+ $tests['no sanitization GET log'] = [$request, ['query' => ['q' => 'index.php']], []];
+
+ $request = new Request([], ['field' => 'value']);
+ $tests['no sanitization POST log'] = [$request, ['request' => ['field' => 'value']], []];
+
+ $request = new Request([], [], [], ['key' => 'value']);
+ $tests['no sanitization COOKIE log'] = [$request, ['cookies' => ['key' => 'value']], []];
+
+ $request = new Request(['#q' => 'index.php']);
+ $tests['sanitization GET'] = [$request];
+
+ $request = new Request([], ['#field' => 'value']);
+ $tests['sanitization POST'] = [$request];
+
+ $request = new Request([], [], [], ['#key' => 'value']);
+ $tests['sanitization COOKIE'] = [$request];
+
+ $request = new Request(['#q' => 'index.php'], ['#field' => 'value'], [], ['#key' => 'value']);
+ $tests['sanitization GET, POST, COOKIE'] = [$request];
+
+ $request = new Request(['#q' => 'index.php']);
+ $tests['sanitization GET log'] = [$request, [], ['Potentially unsafe keys removed from query string parameters (GET): #q']];
+
+ $request = new Request([], ['#field' => 'value']);
+ $tests['sanitization POST log'] = [$request, [], ['Potentially unsafe keys removed from request body parameters (POST): #field']];
+
+ $request = new Request([], [], [], ['#key' => 'value']);
+ $tests['sanitization COOKIE log'] = [$request, [], ['Potentially unsafe keys removed from cookie parameters: #key']];
+
+ $request = new Request(['#q' => 'index.php'], ['#field' => 'value'], [], ['#key' => 'value']);
+ $tests['sanitization GET, POST, COOKIE log'] = [$request, [], ['Potentially unsafe keys removed from query string parameters (GET): #q', 'Potentially unsafe keys removed from request body parameters (POST): #field', 'Potentially unsafe keys removed from cookie parameters: #key']];
+
+ $request = new Request(['q' => 'index.php', 'foo' => ['#bar' => 'foo']]);
+ $tests['recursive sanitization log'] = [$request, ['query' => ['q' => 'index.php', 'foo' => []]], ['Potentially unsafe keys removed from query string parameters (GET): #bar']];
+
+ $request = new Request(['q' => 'index.php', 'foo' => ['#bar' => 'foo']]);
+ $tests['recursive no sanitization whitelist'] = [$request, ['query' => ['q' => 'index.php', 'foo' => ['#bar' => 'foo']]], [], ['#bar']];
+
+ $request = new Request([], ['#field' => 'value']);
+ $tests['no sanitization POST whitelist'] = [$request, ['request' => ['#field' => 'value']], [], ['#field']];
+
+ $request = new Request(['q' => 'index.php', 'foo' => ['#bar' => 'foo', '#foo' => 'bar']]);
+ $tests['recursive multiple sanitization log'] = [$request, ['query' => ['q' => 'index.php', 'foo' => []]], ['Potentially unsafe keys removed from query string parameters (GET): #bar, #foo']];
+
+ $request = new Request(['#q' => 'index.php']);
+ $request->attributes->set(RequestSanitizer::SANITIZED, TRUE);
+ $tests['already sanitized request'] = [$request, ['query' => ['#q' => 'index.php']]];
+
+ $request = new Request(['destination' => 'whatever?%23test=value']);
+ $tests['destination removal GET'] = [$request];
+
+ $request = new Request([], ['destination' => 'whatever?%23test=value']);
+ $tests['destination removal POST'] = [$request];
+
+ $request = new Request([], [], [], ['destination' => 'whatever?%23test=value']);
+ $tests['destination removal COOKIE'] = [$request];
+
+ $request = new Request(['destination' => 'whatever?%23test=value']);
+ $tests['destination removal GET log'] = [$request, [], ['Potentially unsafe destination removed from query parameter bag because it contained the following keys: #test']];
+
+ $request = new Request([], ['destination' => 'whatever?%23test=value']);
+ $tests['destination removal POST log'] = [$request, [], ['Potentially unsafe destination removed from request parameter bag because it contained the following keys: #test']];
+
+ $request = new Request([], [], [], ['destination' => 'whatever?%23test=value']);
+ $tests['destination removal COOKIE log'] = [$request, [], ['Potentially unsafe destination removed from cookies parameter bag because it contained the following keys: #test']];
+
+ $request = new Request(['destination' => 'whatever?q[%23test]=value']);
+ $tests['destination removal subkey'] = [$request];
+
+ $request = new Request(['destination' => 'whatever?q[%23test]=value']);
+ $tests['destination whitelist'] = [$request, ['query' => ['destination' => 'whatever?q[%23test]=value']], [], ['#test']];
+
+ $request = new Request(['destination' => "whatever?\x00bar=base&%23test=value"]);
+ $tests['destination removal zero byte'] = [$request];
+
+ $request = new Request(['destination' => 'whatever?q=value']);
+ $tests['destination kept'] = [$request, ['query' => ['destination' => 'whatever?q=value']]];
+
+ $request = new Request(['destination' => 'whatever']);
+ $tests['destination no query'] = [$request, ['query' => ['destination' => 'whatever']]];
+
+ return $tests;
+ }
+
+ /**
+ * Tests acceptable destinations are not removed from GET requests.
+ *
+ * @param string $destination
+ * The destination string to test.
+ *
+ * @dataProvider providerTestAcceptableDestinations
+ */
+ public function testAcceptableDestinationGet($destination) {
+ // Set up a GET request.
+ $request = $this->createRequestForTesting(['destination' => $destination]);
+
+ $request = RequestSanitizer::sanitize($request, [], TRUE);
+
+ $this->assertSame($destination, $request->query->get('destination', NULL));
+ $this->assertNull($request->request->get('destination', NULL));
+ $this->assertSame($destination, $_GET['destination']);
+ $this->assertSame($destination, $_REQUEST['destination']);
+ $this->assertArrayNotHasKey('destination', $_POST);
+ $this->assertEquals([], $this->errors);
+ }
+
+ /**
+ * Tests unacceptable destinations are removed from GET requests.
+ *
+ * @param string $destination
+ * The destination string to test.
+ *
+ * @dataProvider providerTestSanitizedDestinations
+ */
+ public function testSanitizedDestinationGet($destination) {
+ // Set up a GET request.
+ $request = $this->createRequestForTesting(['destination' => $destination]);
+
+ $request = RequestSanitizer::sanitize($request, [], TRUE);
+
+ $this->assertNull($request->request->get('destination', NULL));
+ $this->assertNull($request->query->get('destination', NULL));
+ $this->assertArrayNotHasKey('destination', $_POST);
+ $this->assertArrayNotHasKey('destination', $_REQUEST);
+ $this->assertArrayNotHasKey('destination', $_GET);
+ $this->assertError('Potentially unsafe destination removed from query parameter bag because it points to an external URL.', E_USER_NOTICE);
+ }
+
+ /**
+ * Tests acceptable destinations are not removed from POST requests.
+ *
+ * @param string $destination
+ * The destination string to test.
+ *
+ * @dataProvider providerTestAcceptableDestinations
+ */
+ public function testAcceptableDestinationPost($destination) {
+ // Set up a POST request.
+ $request = $this->createRequestForTesting([], ['destination' => $destination]);
+
+ $request = RequestSanitizer::sanitize($request, [], TRUE);
+
+ $this->assertSame($destination, $request->request->get('destination', NULL));
+ $this->assertNull($request->query->get('destination', NULL));
+ $this->assertSame($destination, $_POST['destination']);
+ $this->assertSame($destination, $_REQUEST['destination']);
+ $this->assertArrayNotHasKey('destination', $_GET);
+ $this->assertEquals([], $this->errors);
+ }
+
+ /**
+ * Tests unacceptable destinations are removed from GET requests.
+ *
+ * @param string $destination
+ * The destination string to test.
+ *
+ * @dataProvider providerTestSanitizedDestinations
+ */
+ public function testSanitizedDestinationPost($destination) {
+ // Set up a POST request.
+ $request = $this->createRequestForTesting([], ['destination' => $destination]);
+
+ $request = RequestSanitizer::sanitize($request, [], TRUE);
+
+ $this->assertNull($request->request->get('destination', NULL));
+ $this->assertNull($request->query->get('destination', NULL));
+ $this->assertArrayNotHasKey('destination', $_POST);
+ $this->assertArrayNotHasKey('destination', $_REQUEST);
+ $this->assertArrayNotHasKey('destination', $_GET);
+ $this->assertError('Potentially unsafe destination removed from request parameter bag because it points to an external URL.', E_USER_NOTICE);
+ }
+
+ /**
+ * Creates a request and sets PHP globals for testing.
+ *
+ * @param array $query
+ * (optional) The GET parameters.
+ * @param array $request
+ * (optional) The POST parameters.
+ *
+ * @return \Symfony\Component\HttpFoundation\Request
+ * The request object.
+ */
+ protected function createRequestForTesting(array $query = [], array $request = []) {
+ $request = new Request($query, $request);
+
+ // Set up globals.
+ $_GET = $request->query->all();
+ $_POST = $request->request->all();
+ $_COOKIE = $request->cookies->all();
+ $_REQUEST = array_merge($request->query->all(), $request->request->all());
+ $request->server->set('QUERY_STRING', http_build_query($request->query->all()));
+ $_SERVER['QUERY_STRING'] = $request->server->get('QUERY_STRING');
+ return $request;
+ }
+
+ /**
+ * Data provider for testing acceptable destinations.
+ */
+ public function providerTestAcceptableDestinations() {
+ $data = [];
+ // Standard internal example node path is present in the 'destination'
+ // parameter.
+ $data[] = ['node'];
+ // Internal path with one leading slash is allowed.
+ $data[] = ['/example.com'];
+ // Internal URL using a colon is allowed.
+ $data[] = ['example:test'];
+ // Javascript URL is allowed because it is treated as an internal URL.
+ $data[] = ['javascript:alert(0)'];
+ return $data;
+ }
+
+ /**
+ * Data provider for testing sanitized destinations.
+ */
+ public function providerTestSanitizedDestinations() {
+ $data = [];
+ // External URL without scheme is not allowed.
+ $data[] = ['//example.com/test'];
+ // External URL is not allowed.
+ $data[] = ['http://example.com'];
+ $data[] = ['http://example.com//?destination=http://www.evilsite.com%5C@http://www.drupalsite.com'];
+ return $data;
+ }
+
+ /**
+ * Catches and logs errors to $this->errors.
+ *
+ * @param int $errno
+ * The severity level of the error.
+ * @param string $errstr
+ * The error message.
+ */
+ public function errorHandler($errno, $errstr) {
+ $this->errors[] = compact('errno', 'errstr');
+ }
+
+ /**
+ * Asserts that the expected error has been logged.
+ *
+ * @param string $errstr
+ * The error message.
+ * @param int $errno
+ * The severity level of the error.
+ */
+ protected function assertError($errstr, $errno) {
+ foreach ($this->errors as $error) {
+ if ($error['errstr'] === $errstr && $error['errno'] === $errno) {
+ return;
+ }
+ }
+ $this->fail("Error with level $errno and message '$errstr' not found in " . var_export($this->errors, TRUE));
+ }
+
+}
--
GitLab