summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDavid Rothstein2018-02-21 17:28:43 (GMT)
committerDavid Rothstein2018-02-21 17:28:43 (GMT)
commitbc929eb17c4f4eea83cb796b7c5bbb984bf288bd (patch)
tree3c34fd0cf865fff9c4dee580062c3bbf14559914
parent19b0db81b31e7c2f127c79d21e2fe5595946deba (diff)
Drupal 7.57 (SA-CORE-2018-001) by cashwilliams, catch, cilefen, droplet, dawehner, bonus, agentrickard, David_Rothstein, Chi, Gábor Hojtsy, Heine, Wim Leers, Schnitzel, drpal, sugaroverflow, effulgentsia, tedbow, tim.plunkett, tstoeckler, xjm, will_c, stefan.r, samuel.mortenson, larowlan, greggles, logaritmisk, Berdir, mpdonadio, pwolanin, plach7.57
-rw-r--r--CHANGELOG.txt4
-rw-r--r--includes/bootstrap.inc2
-rw-r--r--includes/common.inc5
-rw-r--r--misc/drupal.js38
-rw-r--r--modules/file/file.module16
-rw-r--r--modules/file/tests/file.test73
-rw-r--r--modules/file/tests/file_module_test.module15
-rw-r--r--modules/simpletest/tests/common.test34
-rw-r--r--modules/simpletest/tests/common_test.module3
9 files changed, 184 insertions, 6 deletions
diff --git a/CHANGELOG.txt b/CHANGELOG.txt
index 5ebbf21..90df089 100644
--- a/CHANGELOG.txt
+++ b/CHANGELOG.txt
@@ -1,4 +1,8 @@
+Drupal 7.57, 2018-02-21
+-----------------------
+- Fixed security issues (multiple vulnerabilities). See SA-CORE-2018-001.
+
Drupal 7.56, 2017-06-21
-----------------------
- Fixed security issues (access bypass). See SA-CORE-2017-003.
diff --git a/includes/bootstrap.inc b/includes/bootstrap.inc
index c06055e..655db6d 100644
--- a/includes/bootstrap.inc
+++ b/includes/bootstrap.inc
@@ -8,7 +8,7 @@
/**
* The current system version.
*/
-define('VERSION', '7.56');
+define('VERSION', '7.57');
/**
* Core API compatibility.
diff --git a/includes/common.inc b/includes/common.inc
index a32930a..d7dc47f 100644
--- a/includes/common.inc
+++ b/includes/common.inc
@@ -2236,8 +2236,11 @@ function url($path = NULL, array $options = array()) {
'prefix' => ''
);
+ // Determine whether this is an external link, but ensure that the current
+ // path is always treated as internal by default (to prevent external link
+ // injection vulnerabilities).
if (!isset($options['external'])) {
- $options['external'] = url_is_external($path);
+ $options['external'] = $path === $_GET['q'] ? FALSE : url_is_external($path);
}
// Preserve the original path before altering or aliasing.
diff --git a/misc/drupal.js b/misc/drupal.js
index d86ea1f..19fbc71 100644
--- a/misc/drupal.js
+++ b/misc/drupal.js
@@ -28,6 +28,42 @@ $.fn.init = function (selector, context, rootjQuery) {
$.fn.init.prototype = jquery_init.prototype;
/**
+ * Pre-filter Ajax requests to guard against XSS attacks.
+ *
+ * See https://github.com/jquery/jquery/issues/2432
+ */
+if ($.ajaxPrefilter) {
+ // For newer versions of jQuery, use an Ajax prefilter to prevent
+ // auto-executing script tags from untrusted domains. This is similar to the
+ // fix that is built in to jQuery 3.0 and higher.
+ $.ajaxPrefilter(function (s) {
+ if (s.crossDomain) {
+ s.contents.script = false;
+ }
+ });
+}
+else if ($.httpData) {
+ // For the version of jQuery that ships with Drupal core, override
+ // jQuery.httpData to prevent auto-detecting "script" data types from
+ // untrusted domains.
+ var jquery_httpData = $.httpData;
+ $.httpData = function (xhr, type, s) {
+ // @todo Consider backporting code from newer jQuery versions to check for
+ // a cross-domain request here, rather than using Drupal.urlIsLocal() to
+ // block scripts from all URLs that are not on the same site.
+ if (!type && !Drupal.urlIsLocal(s.url)) {
+ var content_type = xhr.getResponseHeader('content-type') || '';
+ if (content_type.indexOf('javascript') >= 0) {
+ // Default to a safe data type.
+ type = 'text';
+ }
+ }
+ return jquery_httpData.call(this, xhr, type, s);
+ };
+ $.httpData.prototype = jquery_httpData.prototype;
+}
+
+/**
* Attach all registered behaviors to a page element.
*
* Behaviors are event-triggered actions that attach to page elements, enhancing
@@ -137,7 +173,7 @@ Drupal.detachBehaviors = function (context, settings, trigger) {
*/
Drupal.checkPlain = function (str) {
var character, regex,
- replace = { '&': '&amp;', '"': '&quot;', '<': '&lt;', '>': '&gt;' };
+ replace = { '&': '&amp;', "'": '&#39;', '"': '&quot;', '<': '&lt;', '>': '&gt;' };
str = String(str);
for (character in replace) {
if (replace.hasOwnProperty(character)) {
diff --git a/modules/file/file.module b/modules/file/file.module
index fb5e9b9..1e98f11 100644
--- a/modules/file/file.module
+++ b/modules/file/file.module
@@ -140,7 +140,7 @@ function file_file_download($uri, $field_type = 'file') {
}
// Find out which (if any) fields of this type contain the file.
- $references = file_get_file_references($file, NULL, FIELD_LOAD_CURRENT, $field_type);
+ $references = file_get_file_references($file, NULL, FIELD_LOAD_CURRENT, $field_type, FALSE);
// Stop processing if there are no references in order to avoid returning
// headers for files controlled by other modules. Make an exception for
@@ -1067,11 +1067,18 @@ function file_icon_map($file) {
* @param $field_type
* (optional) The name of a field type. If given, limits the reference check
* to fields of the given type.
+ * @param $check_access
+ * (optional) A boolean that specifies whether the permissions of the current
+ * user should be checked when retrieving references. If FALSE, all
+ * references to the file are returned. If TRUE, only references from
+ * entities that the current user has access to are returned. Defaults to
+ * TRUE for backwards compatibility reasons, but FALSE is recommended for
+ * most situations.
*
* @return
* An integer value.
*/
-function file_get_file_references($file, $field = NULL, $age = FIELD_LOAD_REVISION, $field_type = 'file') {
+function file_get_file_references($file, $field = NULL, $age = FIELD_LOAD_REVISION, $field_type = 'file', $check_access = TRUE) {
$references = drupal_static(__FUNCTION__, array());
$fields = isset($field) ? array($field['field_name'] => $field) : field_info_fields();
@@ -1082,6 +1089,11 @@ function file_get_file_references($file, $field = NULL, $age = FIELD_LOAD_REVISI
$query
->fieldCondition($file_field, 'fid', $file->fid)
->age($age);
+ if (!$check_access) {
+ // Neutralize the 'entity_field_access' query tag added by
+ // field_sql_storage_field_storage_query().
+ $query->addTag('DANGEROUS_ACCESS_CHECK_OPT_OUT');
+ }
$references[$field_name] = $query->execute();
}
}
diff --git a/modules/file/tests/file.test b/modules/file/tests/file.test
index b3a1424..f764a90 100644
--- a/modules/file/tests/file.test
+++ b/modules/file/tests/file.test
@@ -1626,6 +1626,79 @@ class FilePrivateTestCase extends FileFieldTestCase {
$this->drupalGet($file_url);
$this->assertResponse(403, 'Confirmed that another anonymous user cannot access the permanent file when it is referenced by an unpublished node.');
}
+
+ /**
+ * Tests file access for private nodes when file download access is granted.
+ */
+ function testPrivateFileDownloadAccessGranted() {
+ // Tell file_module_test to attempt to grant access to all private files,
+ // and ensure that it is doing so correctly.
+ $test_file = $this->getTestFile('text');
+ $uri = file_unmanaged_move($test_file->uri, 'private://');
+ $file_url = file_create_url($uri);
+ $this->drupalGet($file_url);
+ $this->assertResponse(403, 'Access is not granted to an arbitrary private file by default.');
+ variable_set('file_module_test_grant_download_access', TRUE);
+ $this->drupalGet($file_url);
+ $this->assertResponse(200, 'Access is granted to an arbitrary private file after a module grants access to all private files in hook_file_download().');
+
+ // Create a public node with a file attached.
+ $type_name = 'page';
+ $field_name = strtolower($this->randomName());
+ $this->createFileField($field_name, $type_name, array('uri_scheme' => 'private'));
+ $test_file = $this->getTestFile('text');
+ $nid = $this->uploadNodeFile($test_file, $field_name, $type_name, TRUE, array('private' => FALSE));
+ $node = node_load($nid, NULL, TRUE);
+ $file_url = file_create_url($node->{$field_name}[LANGUAGE_NONE][0]['uri']);
+
+ // Unpublish the node and ensure that only administrators (not anonymous
+ // users) can access the node and download the file; the expectation is
+ // that the File module's hook_file_download() implementation will deny
+ // access and thereby override the file_module_test module's access grant.
+ $node->status = NODE_NOT_PUBLISHED;
+ node_save($node);
+ $this->drupalLogin($this->admin_user);
+ $this->drupalGet("node/$nid");
+ $this->assertResponse(200, 'Administrator can access the unpublished node.');
+ $this->drupalGet($file_url);
+ $this->assertResponse(200, 'Administrator can download the file attached to the unpublished node.');
+ $this->drupalLogOut();
+ $this->drupalGet("node/$nid");
+ $this->assertResponse(403, 'Anonymous user cannot access the unpublished node.');
+ $this->drupalGet($file_url);
+ $this->assertResponse(403, 'Anonymous user cannot download the file attached to the unpublished node.');
+
+ // Re-publish the node and ensure that the node and file can be accessed by
+ // everyone.
+ $node->status = NODE_PUBLISHED;
+ node_save($node);
+ $this->drupalLogin($this->admin_user);
+ $this->drupalGet("node/$nid");
+ $this->assertResponse(200, 'Administrator can access the published node.');
+ $this->drupalGet($file_url);
+ $this->assertResponse(200, 'Administrator can download the file attached to the published node.');
+ $this->drupalLogOut();
+ $this->drupalGet("node/$nid");
+ $this->assertResponse(200, 'Anonymous user can access the published node.');
+ $this->drupalGet($file_url);
+ $this->assertResponse(200, 'Anonymous user can download the file attached to the published node.');
+
+ // Make the node private via the node access system and test that only
+ // administrators (not anonymous users) can access the node and download
+ // the file.
+ $node->private = TRUE;
+ node_save($node);
+ $this->drupalLogin($this->admin_user);
+ $this->drupalGet("node/$nid");
+ $this->assertResponse(200, 'Administrator can access the private node.');
+ $this->drupalGet($file_url);
+ $this->assertResponse(200, 'Administrator can download the file attached to the private node.');
+ $this->drupalLogOut();
+ $this->drupalGet("node/$nid");
+ $this->assertResponse(403, 'Anonymous user cannot access the private node.');
+ $this->drupalGet($file_url);
+ $this->assertResponse(403, 'Anonymous user cannot download the file attached to the private node.');
+ }
}
/**
diff --git a/modules/file/tests/file_module_test.module b/modules/file/tests/file_module_test.module
index f66c749..1acfaca 100644
--- a/modules/file/tests/file_module_test.module
+++ b/modules/file/tests/file_module_test.module
@@ -67,3 +67,18 @@ function file_module_test_form_submit($form, &$form_state) {
}
drupal_set_message(t('The file id is %fid.', array('%fid' => $fid)));
}
+
+/**
+ * Implements hook_file_download().
+ */
+function file_module_test_file_download($uri) {
+ if (variable_get('file_module_test_grant_download_access')) {
+ // Mimic what file_get_content_headers() would do if we had a full $file
+ // object to pass to it.
+ return array(
+ 'Content-Type' => mime_header_encode(file_get_mimetype($uri)),
+ 'Content-Length' => filesize($uri),
+ 'Cache-Control' => 'private',
+ );
+ }
+}
diff --git a/modules/simpletest/tests/common.test b/modules/simpletest/tests/common.test
index 83161fa..0490dd5 100644
--- a/modules/simpletest/tests/common.test
+++ b/modules/simpletest/tests/common.test
@@ -76,7 +76,7 @@ class DrupalAlterTestCase extends DrupalWebTestCase {
class CommonURLUnitTest extends DrupalWebTestCase {
public static function getInfo() {
return array(
- 'name' => 'URL generation tests',
+ 'name' => 'URL generation unit tests',
'description' => 'Confirm that url(), drupal_get_query_parameters(), drupal_http_build_query(), and l() work correctly with various input.',
'group' => 'System',
);
@@ -373,6 +373,38 @@ class CommonURLUnitTest extends DrupalWebTestCase {
}
/**
+ * Web tests for URL generation functions.
+ */
+class CommonURLWebTest extends DrupalWebTestCase {
+ public static function getInfo() {
+ return array(
+ 'name' => 'URL generation web tests',
+ 'description' => 'Confirm that URL-generating functions work correctly on specific site paths.',
+ 'group' => 'System',
+ );
+ }
+
+ function setUp() {
+ parent::setUp('common_test');
+ }
+
+ /**
+ * Tests the url() function on internal paths which mimic external URLs.
+ */
+ function testInternalPathMimicsExternal() {
+ // Ensure that calling url(current_path()) on "/http://example.com" (an
+ // internal path which mimics an external URL) always links to the internal
+ // path, not the external URL. This helps protect against external URL link
+ // injection vulnerabilities.
+ variable_set('common_test_link_to_current_path', TRUE);
+ $this->drupalGet('/http://example.com');
+ $this->clickLink('link which should point to the current path');
+ $this->assertUrl('/http://example.com');
+ $this->assertText('link which should point to the current path');
+ }
+}
+
+/**
* Tests url_is_external().
*/
class UrlIsExternalUnitTest extends DrupalUnitTestCase {
diff --git a/modules/simpletest/tests/common_test.module b/modules/simpletest/tests/common_test.module
index 2eb8cd5..d092c92 100644
--- a/modules/simpletest/tests/common_test.module
+++ b/modules/simpletest/tests/common_test.module
@@ -99,6 +99,9 @@ function common_test_init() {
if (variable_get('common_test_redirect_current_path', FALSE)) {
drupal_goto(current_path());
}
+ if (variable_get('common_test_link_to_current_path', FALSE)) {
+ drupal_set_message(l('link which should point to the current path', current_path()));
+ }
}
/**