From ea88f337d41d3c9c9798dc97168d4b2116678fc5 Mon Sep 17 00:00:00 2001 From: Josh Koenig Date: Wed, 23 Jan 2013 09:36:10 -0800 Subject: [PATCH 1/5] Adding version numbers --- modules/pantheon/pantheon_apachesolr/pantheon_apachesolr.info | 2 +- modules/pantheon/pantheon_api/pantheon_api.info | 1 + modules/pantheon/pantheon_login/pantheon_login.info | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/modules/pantheon/pantheon_apachesolr/pantheon_apachesolr.info b/modules/pantheon/pantheon_apachesolr/pantheon_apachesolr.info index f381b2b01..5339071a5 100644 --- a/modules/pantheon/pantheon_apachesolr/pantheon_apachesolr.info +++ b/modules/pantheon/pantheon_apachesolr/pantheon_apachesolr.info @@ -7,4 +7,4 @@ files[] = Pantheon_Apache_Solr_Service.php files[] = Pantheon_Search_Api_Solr_Service.php core = 7.x php = 5.2 -varsion = "7.x-beta1" +varsion = "7.x-rc3" diff --git a/modules/pantheon/pantheon_api/pantheon_api.info b/modules/pantheon/pantheon_api/pantheon_api.info index 93d1726c1..a20c938f3 100644 --- a/modules/pantheon/pantheon_api/pantheon_api.info +++ b/modules/pantheon/pantheon_api/pantheon_api.info @@ -1,6 +1,7 @@ ; $Id$ name = Pantheon Platform API description = Integration with the Pantheon Platform +version = 7.0 package = Pantheon core = 7.x php = 5.2 diff --git a/modules/pantheon/pantheon_login/pantheon_login.info b/modules/pantheon/pantheon_login/pantheon_login.info index ca1ca4a78..e3cd069fd 100644 --- a/modules/pantheon/pantheon_login/pantheon_login.info +++ b/modules/pantheon/pantheon_login/pantheon_login.info @@ -3,6 +3,7 @@ name = Pantheon Login description = Allows users to log in as user #1 from their Pantheon Control Panel package = Pantheon dependencies[] = pantheon_api +version = 7.0 hidden = TRUE core = 7.x php = 5.2 From 1bf9701b7287fbabc9dcdb31a3effa15f505af7c Mon Sep 17 00:00:00 2001 From: Josh Koenig Date: Thu, 31 Jan 2013 16:48:32 -0800 Subject: [PATCH 2/5] Version numbers FTW --- modules/pantheon/pantheon_apachesolr/pantheon_apachesolr.info | 3 ++- modules/pantheon/pantheon_api/pantheon_api.info | 2 +- modules/pantheon/pantheon_login/pantheon_login.info | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/modules/pantheon/pantheon_apachesolr/pantheon_apachesolr.info b/modules/pantheon/pantheon_apachesolr/pantheon_apachesolr.info index 5339071a5..f9caa54fa 100644 --- a/modules/pantheon/pantheon_apachesolr/pantheon_apachesolr.info +++ b/modules/pantheon/pantheon_apachesolr/pantheon_apachesolr.info @@ -2,9 +2,10 @@ name = Pantheon Apache Solr description = Allows the use of Pantheon's ApacheSolr infrastructure package = Pantheon +varsion = "7.x-rc3" dependencies[] = pantheon_api files[] = Pantheon_Apache_Solr_Service.php files[] = Pantheon_Search_Api_Solr_Service.php core = 7.x php = 5.2 -varsion = "7.x-rc3" + diff --git a/modules/pantheon/pantheon_api/pantheon_api.info b/modules/pantheon/pantheon_api/pantheon_api.info index a20c938f3..f87535890 100644 --- a/modules/pantheon/pantheon_api/pantheon_api.info +++ b/modules/pantheon/pantheon_api/pantheon_api.info @@ -1,7 +1,7 @@ ; $Id$ name = Pantheon Platform API description = Integration with the Pantheon Platform -version = 7.0 +version = "7.x-1.0" package = Pantheon core = 7.x php = 5.2 diff --git a/modules/pantheon/pantheon_login/pantheon_login.info b/modules/pantheon/pantheon_login/pantheon_login.info index e3cd069fd..bcef6cba9 100644 --- a/modules/pantheon/pantheon_login/pantheon_login.info +++ b/modules/pantheon/pantheon_login/pantheon_login.info @@ -3,7 +3,7 @@ name = Pantheon Login description = Allows users to log in as user #1 from their Pantheon Control Panel package = Pantheon dependencies[] = pantheon_api -version = 7.0 +version = "7.x-1.0" hidden = TRUE core = 7.x php = 5.2 From 3a24da1b40f5e05876ad7775044500b61eb2ed94 Mon Sep 17 00:00:00 2001 From: David Rothstein Date: Wed, 20 Feb 2013 15:32:50 -0500 Subject: [PATCH 3/5] Drupal 7.20 --- CHANGELOG.txt | 5 ++++ includes/bootstrap.inc | 2 +- modules/image/image.module | 54 +++++++++++++++++++++++++++++++++----- modules/image/image.test | 45 ++++++++++++++++++++----------- modules/user/user.test | 2 +- 5 files changed, 83 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 08acf95a7..750aabb24 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -1,3 +1,8 @@ + +Drupal 7.20, 2013-02-20 +----------------------- +- Fixed security issues (denial of service). See SA-CORE-2013-002. + Drupal 7.19, 2013-01-16 ----------------------- - Fixed security issues (multiple vulnerabilities). See SA-CORE-2013-001. diff --git a/includes/bootstrap.inc b/includes/bootstrap.inc index 53f70e1a3..2cfdfe953 100644 --- a/includes/bootstrap.inc +++ b/includes/bootstrap.inc @@ -8,7 +8,7 @@ /** * The current system version. */ -define('VERSION', '7.19'); +define('VERSION', '7.20'); /** * Core API compatibility. diff --git a/modules/image/image.module b/modules/image/image.module index 07f489233..d7178ad7d 100644 --- a/modules/image/image.module +++ b/modules/image/image.module @@ -30,6 +30,11 @@ define('IMAGE_STORAGE_EDITABLE', IMAGE_STORAGE_NORMAL | IMAGE_STORAGE_OVERRIDE); */ define('IMAGE_STORAGE_MODULE', IMAGE_STORAGE_OVERRIDE | IMAGE_STORAGE_DEFAULT); +/** + * The name of the query parameter for image derivative tokens. + */ +define('IMAGE_DERIVATIVE_TOKEN', 'itok'); + // Load all Field module hooks for Image. require_once DRUPAL_ROOT . '/modules/image/image.field.inc'; @@ -766,16 +771,24 @@ function image_style_options($include_empty = TRUE) { * The image style */ function image_style_deliver($style, $scheme) { - // Check that the style is defined and the scheme is valid. - if (!$style || !file_stream_wrapper_valid_scheme($scheme)) { - drupal_exit(); - } - $args = func_get_args(); array_shift($args); array_shift($args); $target = implode('/', $args); + // Check that the style is defined, the scheme is valid, and the image + // derivative token is valid. (Sites which require image derivatives to be + // generated without a token can set the 'image_allow_insecure_derivatives' + // variable to TRUE to bypass the latter check, but this will increase the + // site's vulnerability to denial-of-service attacks.) + $valid = !empty($style) && file_stream_wrapper_valid_scheme($scheme); + if (!variable_get('image_allow_insecure_derivatives', FALSE)) { + $valid = $valid && isset($_GET[IMAGE_DERIVATIVE_TOKEN]) && $_GET[IMAGE_DERIVATIVE_TOKEN] === image_style_path_token($style['name'], $scheme . '://' . $target); + } + if (!$valid) { + return MENU_ACCESS_DENIED; + } + $image_uri = $scheme . '://' . $target; $derivative_uri = image_style_path($style['name'], $image_uri); @@ -960,6 +973,10 @@ function image_style_flush($style) { */ function image_style_url($style_name, $path) { $uri = image_style_path($style_name, $path); + // The token query is added even if the 'image_allow_insecure_derivatives' + // variable is TRUE, so that the emitted links remain valid if it is changed + // back to the default FALSE. + $token_query = array(IMAGE_DERIVATIVE_TOKEN => image_style_path_token($style_name, $path)); // If not using clean URLs, the image derivative callback is only available // with the query string. If the file does not exist, use url() to ensure @@ -967,10 +984,33 @@ function image_style_url($style_name, $path) { // actual file path, this avoids bootstrapping PHP once the files are built. if (!variable_get('clean_url') && file_uri_scheme($uri) == 'public' && !file_exists($uri)) { $directory_path = file_stream_wrapper_get_instance_by_uri($uri)->getDirectoryPath(); - return url($directory_path . '/' . file_uri_target($uri), array('absolute' => TRUE)); + return url($directory_path . '/' . file_uri_target($uri), array('absolute' => TRUE, 'query' => $token_query)); } - return file_create_url($uri); + $file_url = file_create_url($uri); + // Append the query string with the token. + return $file_url . (strpos($file_url, '?') !== FALSE ? '&' : '?') . drupal_http_build_query($token_query); +} + +/** + * Generates a token to protect an image style derivative. + * + * This prevents unauthorized generation of an image style derivative, + * which can be costly both in CPU time and disk space. + * + * @param $style_name + * The name of the image style. + * @param $uri + * The URI of the image for this style, for example as returned by + * image_style_path(). + * + * @return + * An eight-character token which can be used to protect image style + * derivatives against denial-of-service attacks. + */ +function image_style_path_token($style_name, $uri) { + // Return the first eight characters. + return substr(drupal_hmac_base64($style_name . ':' . $uri, drupal_get_private_key() . drupal_get_hash_salt()), 0, 8); } /** diff --git a/modules/image/image.test b/modules/image/image.test index 1ca846506..d4db2130b 100644 --- a/modules/image/image.test +++ b/modules/image/image.test @@ -192,13 +192,19 @@ class ImageStylesPathAndUrlTestCase extends DrupalWebTestCase { $this->assertNotIdentical(FALSE, $original_uri, t('Created the generated image file.')); // Get the URL of a file that has not been generated and try to create it. - $generated_uri = $scheme . '://styles/' . $this->style_name . '/' . $scheme . '/'. drupal_basename($original_uri); + $generated_uri = image_style_path($this->style_name, $original_uri); $this->assertFalse(file_exists($generated_uri), t('Generated file does not exist.')); $generate_url = image_style_url($this->style_name, $original_uri); if (!$clean_url) { $this->assertTrue(strpos($generate_url, '?q=') !== FALSE, 'When using non-clean URLS, the system path contains the query string.'); } + // Add some extra chars to the token. + $this->drupalGet(str_replace(IMAGE_DERIVATIVE_TOKEN . '=', IMAGE_DERIVATIVE_TOKEN . '=Zo', $generate_url)); + $this->assertResponse(403, 'Image was inaccessible at the URL wih an invalid token.'); + // Change the parameter name so the token is missing. + $this->drupalGet(str_replace(IMAGE_DERIVATIVE_TOKEN . '=', 'wrongparam=', $generate_url)); + $this->assertResponse(403, 'Image was inaccessible at the URL wih a missing token.'); // Fetch the URL that generates the file. $this->drupalGet($generate_url); @@ -238,6 +244,11 @@ class ImageStylesPathAndUrlTestCase extends DrupalWebTestCase { $this->assertNoRaw( chr(137) . chr(80) . chr(78) . chr(71) . chr(13) . chr(10) . chr(26) . chr(10), 'No PNG signature found in the response body.'); } } + elseif ($clean_url) { + // Add some extra chars to the token. + $this->drupalGet(str_replace(IMAGE_DERIVATIVE_TOKEN . '=', IMAGE_DERIVATIVE_TOKEN . '=Zo', $generate_url)); + $this->assertResponse(200, 'Existing image was accessible at the URL wih an invalid token.'); + } } } @@ -661,7 +672,7 @@ class ImageAdminStylesUnitTest extends ImageFieldTestCase { // Test that image is displayed using newly created style. $this->drupalGet('node/' . $nid); - $this->assertRaw(image_style_url($style_name, $node->{$field_name}[LANGUAGE_NONE][0]['uri']), t('Image displayed using style @style.', array('@style' => $style_name))); + $this->assertRaw(check_plain(image_style_url($style_name, $node->{$field_name}[LANGUAGE_NONE][0]['uri'])), t('Image displayed using style @style.', array('@style' => $style_name))); // Rename the style and make sure the image field is updated. $new_style_name = strtolower($this->randomName(10)); @@ -671,7 +682,7 @@ class ImageAdminStylesUnitTest extends ImageFieldTestCase { $this->drupalPost('admin/config/media/image-styles/edit/' . $style_name, $edit, t('Update style')); $this->assertText(t('Changes to the style have been saved.'), t('Style %name was renamed to %new_name.', array('%name' => $style_name, '%new_name' => $new_style_name))); $this->drupalGet('node/' . $nid); - $this->assertRaw(image_style_url($new_style_name, $node->{$field_name}[LANGUAGE_NONE][0]['uri']), t('Image displayed using style replacement style.')); + $this->assertRaw(check_plain(image_style_url($new_style_name, $node->{$field_name}[LANGUAGE_NONE][0]['uri'])), t('Image displayed using style replacement style.')); // Delete the style and choose a replacement style. $edit = array( @@ -682,7 +693,7 @@ class ImageAdminStylesUnitTest extends ImageFieldTestCase { $this->assertRaw($message, $message); $this->drupalGet('node/' . $nid); - $this->assertRaw(image_style_url('thumbnail', $node->{$field_name}[LANGUAGE_NONE][0]['uri']), t('Image displayed using style replacement style.')); + $this->assertRaw(check_plain(image_style_url('thumbnail', $node->{$field_name}[LANGUAGE_NONE][0]['uri'])), t('Image displayed using style replacement style.')); } } @@ -775,7 +786,9 @@ class ImageFieldDisplayTestCase extends ImageFieldTestCase { // Ensure the derivative image is generated so we do not have to deal with // image style callback paths. $this->drupalGet(image_style_url('thumbnail', $image_uri)); - $image_info['path'] = image_style_path('thumbnail', $image_uri); + // Need to create the URL again since it will change if clean URLs + // are disabled. + $image_info['path'] = image_style_url('thumbnail', $image_uri); $image_info['width'] = 100; $image_info['height'] = 50; $default_output = theme('image', $image_info); @@ -1061,7 +1074,7 @@ class ImageDimensionsTestCase extends DrupalWebTestCase { image_effect_save($effect); $img_tag = theme_image_style($variables); - $this->assertEqual($img_tag, '', t('Expected img tag was found.')); + $this->assertEqual($img_tag, '', t('Expected img tag was found.')); $this->assertFalse(file_exists($generated_uri), t('Generated file does not exist.')); $this->drupalGet($url); $this->assertResponse(200, t('Image was generated at the URL.')); @@ -1082,7 +1095,7 @@ class ImageDimensionsTestCase extends DrupalWebTestCase { image_effect_save($effect); $img_tag = theme_image_style($variables); - $this->assertEqual($img_tag, '', t('Expected img tag was found.')); + $this->assertEqual($img_tag, '', t('Expected img tag was found.')); $this->assertFalse(file_exists($generated_uri), t('Generated file does not exist.')); $this->drupalGet($url); $this->assertResponse(200, t('Image was generated at the URL.')); @@ -1104,7 +1117,7 @@ class ImageDimensionsTestCase extends DrupalWebTestCase { image_effect_save($effect); $img_tag = theme_image_style($variables); - $this->assertEqual($img_tag, '', t('Expected img tag was found.')); + $this->assertEqual($img_tag, '', t('Expected img tag was found.')); $this->assertFalse(file_exists($generated_uri), t('Generated file does not exist.')); $this->drupalGet($url); $this->assertResponse(200, t('Image was generated at the URL.')); @@ -1126,7 +1139,7 @@ class ImageDimensionsTestCase extends DrupalWebTestCase { image_effect_save($effect); $img_tag = theme_image_style($variables); - $this->assertEqual($img_tag, '', t('Expected img tag was found.')); + $this->assertEqual($img_tag, '', t('Expected img tag was found.')); $this->assertFalse(file_exists($generated_uri), t('Generated file does not exist.')); $this->drupalGet($url); $this->assertResponse(200, t('Image was generated at the URL.')); @@ -1144,7 +1157,7 @@ class ImageDimensionsTestCase extends DrupalWebTestCase { image_effect_save($effect); $img_tag = theme_image_style($variables); - $this->assertEqual($img_tag, '', t('Expected img tag was found.')); + $this->assertEqual($img_tag, '', t('Expected img tag was found.')); $this->assertFalse(file_exists($generated_uri), t('Generated file does not exist.')); $this->drupalGet($url); $this->assertResponse(200, t('Image was generated at the URL.')); @@ -1165,7 +1178,7 @@ class ImageDimensionsTestCase extends DrupalWebTestCase { image_effect_save($effect); $img_tag = theme_image_style($variables); - $this->assertEqual($img_tag, '', t('Expected img tag was found.')); + $this->assertEqual($img_tag, '', t('Expected img tag was found.')); $this->assertFalse(file_exists($generated_uri), t('Generated file does not exist.')); $this->drupalGet($url); $this->assertResponse(200, t('Image was generated at the URL.')); @@ -1185,7 +1198,7 @@ class ImageDimensionsTestCase extends DrupalWebTestCase { image_effect_save($effect); $img_tag = theme_image_style($variables); - $this->assertEqual($img_tag, '', t('Expected img tag was found.')); + $this->assertEqual($img_tag, '', t('Expected img tag was found.')); $this->assertFalse(file_exists($generated_uri), t('Generated file does not exist.')); $this->drupalGet($url); $this->assertResponse(200, t('Image was generated at the URL.')); @@ -1206,7 +1219,7 @@ class ImageDimensionsTestCase extends DrupalWebTestCase { $effect = image_effect_save($effect); $img_tag = theme_image_style($variables); - $this->assertEqual($img_tag, '', t('Expected img tag was found.')); + $this->assertEqual($img_tag, '', t('Expected img tag was found.')); $this->assertFalse(file_exists($generated_uri), t('Generated file does not exist.')); $this->drupalGet($url); $this->assertResponse(200, t('Image was generated at the URL.')); @@ -1224,7 +1237,7 @@ class ImageDimensionsTestCase extends DrupalWebTestCase { image_effect_save($effect); $img_tag = theme_image_style($variables); - $this->assertEqual($img_tag, '', t('Expected img tag was found.')); + $this->assertEqual($img_tag, '', t('Expected img tag was found.')); } } @@ -1642,7 +1655,7 @@ class ImageThemeFunctionWebTestCase extends DrupalWebTestCase { ), ); $rendered_element = render($element); - $expected_result = ''; + $expected_result = ''; $this->assertEqual($expected_result, $rendered_element, 'theme_image_formatter() correctly renders without title, alt, or path options.'); // Link the image to a fragment on the page, and not a full URL. @@ -1653,7 +1666,7 @@ class ImageThemeFunctionWebTestCase extends DrupalWebTestCase { 'fragment' => $fragment, ); $rendered_element = render($element); - $expected_result = ''; + $expected_result = ''; $this->assertEqual($expected_result, $rendered_element, 'theme_image_formatter() correctly renders a link fragment.'); } diff --git a/modules/user/user.test b/modules/user/user.test index 123beee6c..26e93ed3c 100644 --- a/modules/user/user.test +++ b/modules/user/user.test @@ -968,7 +968,7 @@ class UserPictureTestCase extends DrupalWebTestCase { $this->assertRaw($text, t('Image was resized.')); $alt = t("@user's picture", array('@user' => format_username($this->user))); $style = variable_get('user_picture_style', ''); - $this->assertRaw(image_style_url($style, $pic_path), t("Image is displayed in user's edit page")); + $this->assertRaw(check_plain(image_style_url($style, $pic_path)), t("Image is displayed in user's edit page")); // Check if file is located in proper directory. $this->assertTrue(is_file($pic_path), t("File is located in proper directory")); From 53098cf059236110716ca97c18565a178c458b43 Mon Sep 17 00:00:00 2001 From: Elliott Foster Date: Thu, 21 Feb 2013 10:45:18 -0600 Subject: [PATCH 4/5] Add hook_js_cache_alter(). --- includes/common.inc | 4 ++++ modules/system/system.api.php | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/includes/common.inc b/includes/common.inc index 0a3fc5366..973b35a40 100644 --- a/includes/common.inc +++ b/includes/common.inc @@ -4929,6 +4929,10 @@ function drupal_build_js_cache($files) { $contents .= file_get_contents($path) . ";\n"; } } + + // Allow modules to act on the js_cche before writing to disk. + drupal_alter('js_cache', $contents); + // Prefix filename to prevent blocking by firewalls which reject files // starting with "ad*". $filename = 'js_' . drupal_hash_base64($contents) . '.js'; diff --git a/modules/system/system.api.php b/modules/system/system.api.php index adef26141..829a673b1 100644 --- a/modules/system/system.api.php +++ b/modules/system/system.api.php @@ -744,6 +744,26 @@ function hook_js_alter(&$javascript) { $javascript['misc/jquery.js']['data'] = drupal_get_path('module', 'jquery_update') . '/jquery.js'; } +/** + * Perform necessary alterations to the concatenated JavaScript before it is + * presented on the page. + * + * @param $contents + * A string of the concatenated JavaScript. + * + * @see drupal_build_js_cache() + */ +function hook_js_cache_alter(&$contents) { + $header = <<
Date: Thu, 21 Feb 2013 15:52:56 -0600 Subject: [PATCH 5/5] Fixing comment typo. --- includes/common.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/common.inc b/includes/common.inc index 973b35a40..9af767379 100644 --- a/includes/common.inc +++ b/includes/common.inc @@ -4930,7 +4930,7 @@ function drupal_build_js_cache($files) { } } - // Allow modules to act on the js_cche before writing to disk. + // Allow modules to act on the js_cache before writing to disk. drupal_alter('js_cache', $contents); // Prefix filename to prevent blocking by firewalls which reject files