Created
April 26, 2018 05:50
-
-
Save popstas/93c9c5c1a61c479d564deb0b1bc380c4 to your computer and use it in GitHub Desktop.
SA-CORE-2018-004 for Drupal 7.34 or older
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/includes/bootstrap.inc b/includes/bootstrap.inc | |
index 06acf93..d5963a0 100644 | |
--- a/includes/bootstrap.inc | |
+++ b/includes/bootstrap.inc | |
@@ -2778,6 +2778,31 @@ function _drupal_bootstrap_variables() { | |
// Load bootstrap modules. | |
require_once DRUPAL_ROOT . '/includes/module.inc'; | |
module_load_all(TRUE); | |
+ | |
+ // 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. | |
+ if (isset($_GET['destination']) || isset($_REQUEST['destination'])) { | |
+ require_once DRUPAL_ROOT . '/includes/common.inc'; | |
+ // If the destination is an external URL, remove it. | |
+ if (isset($_GET['destination']) && url_is_external($_GET['destination'])) { | |
+ 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'])) { | |
+ unset($_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'); |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment