From f818dfe90847f350167055f6207befdc2e4e0f14 Mon Sep 17 00:00:00 2001 From: Dries Buytaert Date: Thu, 14 Jan 2010 18:45:17 +0000 Subject: - Patch #590656 by pwolanin, Pasqualle: harden one-time login links against vulnerability from disclosure of SQL backups, or SQL 'SELECT' injection. --- includes/bootstrap.inc | 2 +- includes/common.inc | 19 +++++++++++++++++-- includes/update.inc | 3 ++- install.php | 4 ++++ modules/user/user.module | 3 ++- sites/default/default.settings.php | 19 +++++++++++++++++++ 6 files changed, 45 insertions(+), 5 deletions(-) diff --git a/includes/bootstrap.inc b/includes/bootstrap.inc index 7bc180ef5..eddfd555a 100644 --- a/includes/bootstrap.inc +++ b/includes/bootstrap.inc @@ -532,7 +532,7 @@ function drupal_settings_initialize() { global $base_url, $base_path, $base_root; // Export the following settings.php variables to the global namespace - global $databases, $db_prefix, $cookie_domain, $conf, $installed_profile, $update_free_access, $db_url, $is_https, $base_secure_url, $base_insecure_url; + global $databases, $db_prefix, $cookie_domain, $conf, $installed_profile, $update_free_access, $db_url, $drupal_hash_salt, $is_https, $base_secure_url, $base_insecure_url; $conf = array(); if (file_exists(DRUPAL_ROOT . '/' . conf_path() . '/settings.php')) { diff --git a/includes/common.inc b/includes/common.inc index 75dbe5685..e04c71303 100644 --- a/includes/common.inc +++ b/includes/common.inc @@ -4460,6 +4460,19 @@ function drupal_random_bytes($count) { return substr($output, 0, $count); } +/** + * Get a salt useful for hardening against SQL injection. + * + * @return + * A salt based on information in settings.php, not in the database. + */ +function drupal_get_hash_salt() { + global $drupal_hash_salt, $databases; + // If the $drupal_hash_salt variable is empty, a hash of the serialized + // database credentials is used as a fallback salt. + return empty($drupal_hash_salt) ? sha1(serialize($databases)) : $drupal_hash_salt; +} + /** * Ensure the private key variable used to generate tokens is set. * @@ -4482,7 +4495,9 @@ function drupal_get_private_key() { */ function drupal_get_token($value = '') { $private_key = drupal_get_private_key(); - return md5(session_id() . $value . $private_key); + // A single md5() is vulnerable to length-extension attacks, so use it twice. + // @todo: add md5 and sha1 hmac functions to core. + return md5(drupal_get_hash_salt() . md5(session_id() . $value . $private_key)); } /** @@ -4500,7 +4515,7 @@ function drupal_get_token($value = '') { */ function drupal_valid_token($token, $value = '', $skip_anonymous = FALSE) { global $user; - return (($skip_anonymous && $user->uid == 0) || ($token == md5(session_id() . $value . variable_get('drupal_private_key', '')))); + return (($skip_anonymous && $user->uid == 0) || ($token == drupal_get_token($value))); } function _drupal_bootstrap_full() { diff --git a/includes/update.inc b/includes/update.inc index 805857c43..a7f3f64b6 100644 --- a/includes/update.inc +++ b/includes/update.inc @@ -262,7 +262,8 @@ function update_fix_d7_requirements() { global $update_rewrite_settings, $db_url; if (!empty($update_rewrite_settings)) { $databases = update_parse_db_url($db_url); - file_put_contents(conf_path() . '/settings.php', "\n" . '$databases = ' . var_export($databases, TRUE) . ';', FILE_APPEND); + $salt = sha1(drupal_random_bytes(64)); + file_put_contents(conf_path() . '/settings.php', "\n" . '$databases = ' . var_export($databases, TRUE) . ";\n\$drupal_hash_salt = '$salt';", FILE_APPEND); } if (drupal_get_installed_schema_version('system') < 7000 && !variable_get('update_d7_requirements', FALSE)) { // Add the cache_path table. diff --git a/install.php b/install.php index 0b9dddd87..f4da1bb55 100644 --- a/install.php +++ b/install.php @@ -1002,6 +1002,10 @@ function install_settings_form_submit($form, &$form_state) { 'value' => $form_state['values']['db_prefix'], 'required' => TRUE, ); + $settings['drupal_hash_salt'] = array( + 'value' => sha1(drupal_random_bytes(64)), + 'required' => TRUE, + ); drupal_rewrite_settings($settings); // Indicate that the settings file has been verified, and check the database // for the last completed task, now that we have a valid connection. This diff --git a/modules/user/user.module b/modules/user/user.module index ee9e5287f..993b13f58 100644 --- a/modules/user/user.module +++ b/modules/user/user.module @@ -2029,7 +2029,8 @@ function user_cancel_url($account) { } function user_pass_rehash($password, $timestamp, $login) { - return md5($timestamp . $password . $login); + // A single md5() is vulnerable to length-extension attacks, so use it twice. + return md5(drupal_get_hash_salt() . md5($timestamp . $password . $login)); } /** diff --git a/sites/default/default.settings.php b/sites/default/default.settings.php index d8c3c72e8..15c41a725 100644 --- a/sites/default/default.settings.php +++ b/sites/default/default.settings.php @@ -167,6 +167,25 @@ $db_prefix = ''; */ $update_free_access = FALSE; +/** + * Salt for one-time login links and cancel links, form tokens, etc. + * + * This variable will be set to a random value by the installer. All one-time + * login links will be invalidated if the value is changed. Note that this + * variable must have the same value on every web server. If this variable is + * empty, a hash of the serialized database credentials will be used as a + * fallback salt. + * + * For enhanced security, you may set this variable to a value using the + * contents of a file outside your docroot that is never saved together + * with any backups of your Drupal files and database. + * + * Example: + * $drupal_hash_salt = file_get_contents('/home/example/salt.txt'); + * + */ +$drupal_hash_salt = ''; + /** * Base URL (optional). * -- cgit v1.2.3