-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Applying security patch SA-CORE-2018-004.
- Loading branch information
Showing
5 changed files
with
132 additions
and
2 deletions.
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| diff --git a/includes/bootstrap.inc b/includes/bootstrap.inc | ||
| index 06acf93..d5963a0 100644 | ||
| --- a/includes/bootstrap.inc | ||
| +++ b/includes/bootstrap.inc | ||
| @@ -2778,6 +2778,11 @@ function _drupal_bootstrap_variables() { | ||
| unset($_GET['destination']); | ||
| unset($_REQUEST['destination']); | ||
| } | ||
| + // Use the DrupalRequestSanitizer to ensure that the destination's query | ||
| + // parameters are not dangerous. | ||
| + if (isset($_GET['destination'])) { | ||
| + DrupalRequestSanitizer::cleanDestination(); | ||
| + } | ||
| // If there's still something in $_REQUEST['destination'] that didn't come | ||
| // from $_GET, check it too. | ||
| if (isset($_REQUEST['destination']) && (!isset($_GET['destination']) || $_REQUEST['destination'] != $_GET['destination']) && url_is_external($_REQUEST['destination'])) { | ||
| diff --git a/includes/common.inc b/includes/common.inc | ||
| index d7dc47f..f61d1eb 100644 | ||
| --- a/includes/common.inc | ||
| +++ b/includes/common.inc | ||
| @@ -611,8 +611,9 @@ function drupal_parse_url($url) { | ||
| } | ||
| // The 'q' parameter contains the path of the current page if clean URLs are | ||
| // disabled. It overrides the 'path' of the URL when present, even if clean | ||
| - // URLs are enabled, due to how Apache rewriting rules work. | ||
| - if (isset($options['query']['q'])) { | ||
| + // URLs are enabled, due to how Apache rewriting rules work. The path | ||
| + // parameter must be a string. | ||
| + if (isset($options['query']['q']) && is_string($options['query']['q'])) { | ||
| $options['path'] = $options['query']['q']; | ||
| unset($options['query']['q']); | ||
| } | ||
| diff --git a/includes/request-sanitizer.inc b/includes/request-sanitizer.inc | ||
| index 1daa6b5..7214436 100644 | ||
| --- a/includes/request-sanitizer.inc | ||
| +++ b/includes/request-sanitizer.inc | ||
| @@ -52,6 +52,38 @@ class DrupalRequestSanitizer { | ||
| } | ||
|
|
||
| /** | ||
| + * Removes the destination if it is dangerous. | ||
| + * | ||
| + * Note this can only be called after common.inc has been included. | ||
| + * | ||
| + * @return bool | ||
| + * TRUE if the destination has been removed from $_GET, FALSE if not. | ||
| + */ | ||
| + public static function cleanDestination() { | ||
| + $dangerous_keys = array(); | ||
| + $log_sanitized_keys = variable_get('sanitize_input_logging', FALSE); | ||
| + | ||
| + $parts = drupal_parse_url($_GET['destination']); | ||
| + // If there is a query string, check its query parameters. | ||
| + if (!empty($parts['query'])) { | ||
| + $whitelist = variable_get('sanitize_input_whitelist', array()); | ||
| + | ||
| + self::stripDangerousValues($parts['query'], $whitelist, $dangerous_keys); | ||
| + if (!empty($dangerous_keys)) { | ||
| + // The destination is removed rather than sanitized to mirror the | ||
| + // handling of external destinations. | ||
| + unset($_GET['destination']); | ||
| + unset($_REQUEST['destination']); | ||
| + if ($log_sanitized_keys) { | ||
| + trigger_error(format_string('Potentially unsafe destination removed from query string parameters (GET) because it contained the following keys: @keys', array('@keys' => implode(', ', $dangerous_keys)))); | ||
| + } | ||
| + return TRUE; | ||
| + } | ||
| + } | ||
| + return FALSE; | ||
| + } | ||
| + | ||
| + /** | ||
| * Strips dangerous keys from the provided input. | ||
| * | ||
| * @param mixed $input | ||
| diff --git a/modules/file/file.module b/modules/file/file.module | ||
| index 1e98f11..eea5847 100644 | ||
| --- a/modules/file/file.module | ||
| +++ b/modules/file/file.module | ||
| @@ -239,6 +239,9 @@ function file_ajax_upload() { | ||
| $form_parents = func_get_args(); | ||
| $form_build_id = (string) array_pop($form_parents); | ||
|
|
||
| + // Sanitize form parents before using them. | ||
| + $form_parents = array_filter($form_parents, 'element_child'); | ||
| + | ||
| if (empty($_POST['form_build_id']) || $form_build_id != $_POST['form_build_id']) { | ||
| // Invalid request. | ||
| drupal_set_message(t('An unrecoverable error occurred. The uploaded file likely exceeded the maximum file size (@size) that this server supports.', array('@size' => format_size(file_upload_max_size()))), 'error'); |