Created
April 25, 2018 16:42
-
-
Save andyg5000/bb1cf400b8c01555fb70401288879eca to your computer and use it in GitHub Desktop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
diff --git a/core/lib/Drupal/Core/Security/RequestSanitizer.php b/core/lib/Drupal/Core/Security/RequestSanitizer.php | |
index 8ba17b9..44815f6 100644 | |
--- a/core/lib/Drupal/Core/Security/RequestSanitizer.php | |
+++ b/core/lib/Drupal/Core/Security/RequestSanitizer.php | |
@@ -2,6 +2,8 @@ | |
namespace Drupal\Core\Security; | |
+use Drupal\Component\Utility\UrlHelper; | |
+use Symfony\Component\HttpFoundation\ParameterBag; | |
use Symfony\Component\HttpFoundation\Request; | |
/** | |
@@ -39,33 +41,89 @@ class RequestSanitizer { | |
*/ | |
public static function sanitize(Request $request, $whitelist, $log_sanitized_keys = FALSE) { | |
if (!$request->attributes->get(self::SANITIZED, FALSE)) { | |
- // Process query string parameters. | |
- $get_sanitized_keys = []; | |
- $request->query->replace(static::stripDangerousValues($request->query->all(), $whitelist, $get_sanitized_keys)); | |
- if ($log_sanitized_keys && !empty($get_sanitized_keys)) { | |
- trigger_error(sprintf('Potentially unsafe keys removed from query string parameters (GET): %s', implode(', ', $get_sanitized_keys))); | |
+ $update_globals = FALSE; | |
+ $bags = [ | |
+ 'query' => 'Potentially unsafe keys removed from query string parameters (GET): %s', | |
+ 'request' => 'Potentially unsafe keys removed from request body parameters (POST): %s', | |
+ 'cookies' => 'Potentially unsafe keys removed from cookie parameters: %s', | |
+ ]; | |
+ foreach ($bags as $bag => $message) { | |
+ if (static::processParameterBag($request->$bag, $whitelist, $log_sanitized_keys, $bag, $message)) { | |
+ $update_globals = TRUE; | |
+ } | |
} | |
- | |
- // Request body parameters. | |
- $post_sanitized_keys = []; | |
- $request->request->replace(static::stripDangerousValues($request->request->all(), $whitelist, $post_sanitized_keys)); | |
- if ($log_sanitized_keys && !empty($post_sanitized_keys)) { | |
- trigger_error(sprintf('Potentially unsafe keys removed from request body parameters (POST): %s', implode(', ', $post_sanitized_keys))); | |
+ if ($update_globals) { | |
+ $request->overrideGlobals(); | |
} | |
+ $request->attributes->set(self::SANITIZED, TRUE); | |
+ } | |
+ return $request; | |
+ } | |
- // Cookie parameters. | |
- $cookie_sanitized_keys = []; | |
- $request->cookies->replace(static::stripDangerousValues($request->cookies->all(), $whitelist, $cookie_sanitized_keys)); | |
- if ($log_sanitized_keys && !empty($cookie_sanitized_keys)) { | |
- trigger_error(sprintf('Potentially unsafe keys removed from cookie parameters: %s', implode(', ', $cookie_sanitized_keys))); | |
+ /** | |
+ * Processes a request parameter bag. | |
+ * | |
+ * @param \Symfony\Component\HttpFoundation\ParameterBag $bag | |
+ * The parameter bag to process. | |
+ * @param string[] $whitelist | |
+ * An array of keys to whitelist as safe. | |
+ * @param bool $log_sanitized_keys | |
+ * Set to TRUE to log keys that are sanitized. | |
+ * @param string $bag_name | |
+ * The request parameter bag name. Either 'query', 'request' or 'cookies'. | |
+ * @param string $message | |
+ * The message to log if the parameter bag contains keys that are removed. | |
+ * If the message contains %s that is replaced by a list of removed keys. | |
+ * | |
+ * @return bool | |
+ * TRUE if the parameter bag has been sanitized, FALSE if not. | |
+ */ | |
+ protected static function processParameterBag(ParameterBag $bag, $whitelist, $log_sanitized_keys, $bag_name, $message) { | |
+ $sanitized = FALSE; | |
+ $sanitized_keys = []; | |
+ $bag->replace(static::stripDangerousValues($bag->all(), $whitelist, $sanitized_keys)); | |
+ if (!empty($sanitized_keys)) { | |
+ $sanitized = TRUE; | |
+ if ($log_sanitized_keys) { | |
+ trigger_error(sprintf($message, implode(', ', $sanitized_keys))); | |
} | |
+ } | |
- if (!empty($get_sanitized_keys) || !empty($post_sanitized_keys) || !empty($cookie_sanitized_keys)) { | |
- $request->overrideGlobals(); | |
+ if ($bag->has('destination')) { | |
+ $destination_dangerous_keys = static::checkDestination($bag->get('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 | |
+ // early in the bootstrap. | |
+ $bag->remove('destination'); | |
+ $sanitized = TRUE; | |
+ if ($log_sanitized_keys) { | |
+ trigger_error(sprintf('Potentially unsafe destination removed from %s parameter bag because it contained the following keys: %s', $bag_name, implode(', ', $destination_dangerous_keys))); | |
+ } | |
} | |
- $request->attributes->set(self::SANITIZED, TRUE); | |
} | |
- return $request; | |
+ return $sanitized; | |
+ } | |
+ | |
+ /** | |
+ * Checks a destination string to see if it is dangerous. | |
+ * | |
+ * @param string $destination | |
+ * The destination string to check. | |
+ * @param array $whitelist | |
+ * An array of keys to whitelist as safe. | |
+ * | |
+ * @return array | |
+ * The dangerous keys found in the destination parameter. | |
+ */ | |
+ protected static function checkDestination($destination, array $whitelist) { | |
+ $dangerous_keys = []; | |
+ $parts = UrlHelper::parse($destination); | |
+ // If there is a query string, check its query parameters. | |
+ if (!empty($parts['query'])) { | |
+ static::stripDangerousValues($parts['query'], $whitelist, $dangerous_keys); | |
+ } | |
+ return $dangerous_keys; | |
} | |
/** | |
diff --git a/core/modules/file/src/Element/ManagedFile.php b/core/modules/file/src/Element/ManagedFile.php | |
index ca4e887..6f01ee5 100644 | |
--- a/core/modules/file/src/Element/ManagedFile.php | |
+++ b/core/modules/file/src/Element/ManagedFile.php | |
@@ -8,6 +8,7 @@ use Drupal\Component\Utility\NestedArray; | |
use Drupal\Core\Ajax\AjaxResponse; | |
use Drupal\Core\Ajax\ReplaceCommand; | |
use Drupal\Core\Form\FormStateInterface; | |
+use Drupal\Core\Render\Element; | |
use Drupal\Core\Render\Element\FormElement; | |
use Drupal\Core\Site\Settings; | |
use Drupal\Core\Url; | |
@@ -175,6 +176,9 @@ class ManagedFile extends FormElement { | |
$form_parents = explode('/', $request->query->get('element_parents')); | |
+ // Sanitize form parents before using them. | |
+ $form_parents = array_filter($form_parents, [Element::class, 'child']); | |
+ | |
// Retrieve the element to be rendered. | |
$form = NestedArray::getValue($form, $form_parents); | |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment