From 83b80acad8431fcd56e9a331ba06c41edee48c91 Mon Sep 17 00:00:00 2001 From: David Rothstein Date: Wed, 16 Jul 2014 16:03:02 -0400 Subject: Drupal 7.29 --- CHANGELOG.txt | 3 ++ includes/bootstrap.inc | 11 ++++- includes/file.inc | 81 ++++++++++++++++++++++++++------- includes/form.inc | 2 +- misc/ajax.js | 2 +- modules/file/file.module | 5 +- modules/file/tests/file.test | 13 ++++++ modules/simpletest/tests/bootstrap.test | 5 ++ 8 files changed, 99 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index ed7343166..f25e25353 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -1,3 +1,6 @@ +Drupal 7.29, 2014-07-16 +---------------------- +- Fixed security issues (multiple vulnerabilities). See SA-CORE-2014-003. Drupal 7.28, 2014-05-08 ----------------------- diff --git a/includes/bootstrap.inc b/includes/bootstrap.inc index 09c2044bd..d8120cba3 100644 --- a/includes/bootstrap.inc +++ b/includes/bootstrap.inc @@ -8,7 +8,7 @@ /** * The current system version. */ -define('VERSION', '7.28'); +define('VERSION', '7.29'); /** * Core API compatibility. @@ -700,7 +700,14 @@ function drupal_environment_initialize() { * TRUE if only containing valid characters, or FALSE otherwise. */ function drupal_valid_http_host($host) { - return preg_match('/^\[?(?:[a-zA-Z0-9-:\]_]+\.?)+$/', $host); + // Limit the length of the host name to 1000 bytes to prevent DoS attacks with + // long host names. + return strlen($host) <= 1000 + // Limit the number of subdomains and port separators to prevent DoS attacks + // in conf_path(). + && substr_count($host, '.') <= 100 + && substr_count($host, ':') <= 100 + && preg_match('/^\[?(?:[a-zA-Z0-9-:\]_]+\.?)+$/', $host); } /** diff --git a/includes/file.inc b/includes/file.inc index 834699f18..d3008cc4f 100644 --- a/includes/file.inc +++ b/includes/file.inc @@ -1999,23 +1999,7 @@ function file_download() { $target = implode('/', $args); $uri = $scheme . '://' . $target; if (file_stream_wrapper_valid_scheme($scheme) && file_exists($uri)) { - // Let other modules provide headers and controls access to the file. - // module_invoke_all() uses array_merge_recursive() which merges header - // values into a new array. To avoid that and allow modules to override - // headers instead, use array_merge() to merge the returned arrays. - $headers = array(); - foreach (module_implements('file_download') as $module) { - $function = $module . '_file_download'; - $result = $function($uri); - if ($result == -1) { - // Throw away the headers received so far. - $headers = array(); - break; - } - if (isset($result) && is_array($result)) { - $headers = array_merge($headers, $result); - } - } + $headers = file_download_headers($uri); if (count($headers)) { file_transfer($uri, $headers); } @@ -2027,6 +2011,69 @@ function file_download() { drupal_exit(); } +/** + * Retrieves headers for a private file download. + * + * Calls all module implementations of hook_file_download() to retrieve headers + * for files by the module that originally provided the file. The presence of + * returned headers indicates the current user has access to the file. + * + * @param $uri + * The URI for the file whose headers should be retrieved. + * + * @return + * If access is allowed, headers for the file, suitable for passing to + * file_transfer(). If access is not allowed, an empty array will be returned. + * + * @see file_transfer() + * @see file_download_access() + * @see hook_file_downlaod() + */ +function file_download_headers($uri) { + // Let other modules provide headers and control access to the file. + // module_invoke_all() uses array_merge_recursive() which merges header + // values into a new array. To avoid that and allow modules to override + // headers instead, use array_merge() to merge the returned arrays. + $headers = array(); + foreach (module_implements('file_download') as $module) { + $function = $module . '_file_download'; + $result = $function($uri); + if ($result == -1) { + // Throw away the headers received so far. + $headers = array(); + break; + } + if (isset($result) && is_array($result)) { + $headers = array_merge($headers, $result); + } + } + return $headers; +} + +/** + * Checks that the current user has access to a particular file. + * + * The return value of this function hinges on the return value from + * file_download_headers(), which is the function responsible for collecting + * access information through hook_file_download(). + * + * If immediately transferring the file to the browser and the headers will + * need to be retrieved, the return value of file_download_headers() should be + * used to determine access directly, so that access checks will not be run + * twice. + * + * @param $uri + * The URI for the file whose access should be retrieved. + * + * @return + * Boolean TRUE if access is allowed. FALSE if access is not allowed. + * + * @see file_download_headers() + * @see hook_file_download() + */ +function file_download_access($uri) { + return count(file_download_headers($uri)) > 0; +} /** * Finds all files that match a given mask in a given directory. diff --git a/includes/form.inc b/includes/form.inc index 846bcb51f..3840885c1 100644 --- a/includes/form.inc +++ b/includes/form.inc @@ -2722,7 +2722,7 @@ function form_select_options($element, $choices = NULL) { $options = ''; foreach ($choices as $key => $choice) { if (is_array($choice)) { - $options .= ''; + $options .= ''; $options .= form_select_options($element, $choice); $options .= ''; } diff --git a/misc/ajax.js b/misc/ajax.js index 29483b498..3b9dec614 100644 --- a/misc/ajax.js +++ b/misc/ajax.js @@ -348,7 +348,7 @@ Drupal.ajax.prototype.beforeSend = function (xmlhttprequest, options) { // this is only needed for IFRAME submissions. var v = $.fieldValue(this.element); if (v !== null) { - options.extraData[this.element.name] = v; + options.extraData[this.element.name] = Drupal.checkPlain(v); } } diff --git a/modules/file/file.module b/modules/file/file.module index 5a635fd75..ed165368b 100644 --- a/modules/file/file.module +++ b/modules/file/file.module @@ -510,8 +510,9 @@ function file_managed_file_value(&$element, $input = FALSE, $form_state = NULL) $callback($element, $input, $form_state); } } - // Load file if the FID has changed to confirm it exists. - if (isset($input['fid']) && $file = file_load($input['fid'])) { + // Load file and check access if the FID has changed, to confirm it + // exists and that the current user has access to it. + if (isset($input['fid']) && ($file = file_load($input['fid'])) && file_download_access($file->uri)) { $fid = $file->fid; } } diff --git a/modules/file/tests/file.test b/modules/file/tests/file.test index 69e711a36..4d53d747f 100644 --- a/modules/file/tests/file.test +++ b/modules/file/tests/file.test @@ -1167,5 +1167,18 @@ class FilePrivateTestCase extends FileFieldTestCase { // Ensure the file cannot be downloaded. $this->drupalGet(file_create_url($node_file->uri)); $this->assertResponse(403, 'Confirmed that access is denied for the file without view field access permission.'); + + // Attempt to reuse the existing file when creating a new node, and confirm + // that access is still denied. + $edit = array(); + $edit['title'] = $this->randomName(8); + $edit[$field_name . '[' . LANGUAGE_NONE . '][0][fid]'] = $node_file->fid; + $this->drupalPost('node/add/page', $edit, t('Save')); + $new_node = $this->drupalGetNodeByTitle($edit['title']); + $this->assertTrue(!empty($new_node), 'Node was created.'); + $this->assertUrl('node/' . $new_node->nid); + $this->assertNoRaw($node_file->filename, 'File without view field access permission does not appear after attempting to attach it to a new node.'); + $this->drupalGet(file_create_url($node_file->uri)); + $this->assertResponse(403, 'Confirmed that access is denied for the file without view field access permission after attempting to attach it to a new node.'); } } diff --git a/modules/simpletest/tests/bootstrap.test b/modules/simpletest/tests/bootstrap.test index 4fda15c8c..5dcde3258 100644 --- a/modules/simpletest/tests/bootstrap.test +++ b/modules/simpletest/tests/bootstrap.test @@ -93,6 +93,11 @@ class BootstrapIPAddressTestCase extends DrupalWebTestCase { $this->assertFalse(drupal_valid_http_host('security\\.drupal.org:80'), 'HTTP_HOST with \\ is invalid'); $this->assertFalse(drupal_valid_http_host('security<.drupal.org:80'), 'HTTP_HOST with < is invalid'); $this->assertFalse(drupal_valid_http_host('security..drupal.org:80'), 'HTTP_HOST with .. is invalid'); + // Verifies that host names are shorter than 1000 characters. + $this->assertFalse(drupal_valid_http_host(str_repeat('x', 1001)), 'HTTP_HOST with more than 1000 characters is invalid.'); + $this->assertFalse(drupal_valid_http_host(str_repeat('.', 101)), 'HTTP_HOST with more than 100 subdomains is invalid.'); + $this->assertFalse(drupal_valid_http_host(str_repeat(':', 101)), 'HTTP_HOST with more than 100 portseparators is invalid.'); + // IPv6 loopback address $this->assertTrue(drupal_valid_http_host('[::1]:80'), 'HTTP_HOST containing IPv6 loopback is valid'); } -- cgit v1.2.3