From d652267155a05f7ebe25f2c5687c28b59b1ab546 Mon Sep 17 00:00:00 2001 From: David Rothstein Date: Wed, 7 Jun 2017 18:14:15 -0400 Subject: [PATCH 1/2] Back to 7.x-dev --- CHANGELOG.txt | 3 +++ includes/bootstrap.inc | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 6a20156af04..600f875b4e5 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -1,4 +1,7 @@ +Drupal 7.xx, xxxx-xx-xx (development version) +----------------------- + Drupal 7.55, 2017-06-07 ----------------------- - Fixed incompatibility with PHP versions 7.0.19 and 7.1.5 due to duplicate diff --git a/includes/bootstrap.inc b/includes/bootstrap.inc index d5864ff0bfc..4bc9df3c081 100644 --- a/includes/bootstrap.inc +++ b/includes/bootstrap.inc @@ -8,7 +8,7 @@ /** * The current system version. */ -define('VERSION', '7.55'); +define('VERSION', '7.56-dev'); /** * Core API compatibility. From 19b0db81b31e7c2f127c79d21e2fe5595946deba Mon Sep 17 00:00:00 2001 From: David Rothstein Date: Wed, 21 Jun 2017 14:20:18 -0400 Subject: [PATCH 2/2] Drupal 7.56 (SA-CORE-2017-003) by alexpott, catch, cilefen, David_Rothstein, dokumori, greggles, iancawthorne, larowlan, mlhess, pwolanin, quicksketch, smaz, stefan.r, xjm --- CHANGELOG.txt | 3 +- includes/bootstrap.inc | 2 +- includes/file.inc | 14 +++++++ modules/file/file.module | 5 ++- modules/file/tests/file.test | 74 ++++++++++++++++++++++++++++++++++++ 5 files changed, 94 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 600f875b4e5..5ebbf2117cc 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -1,6 +1,7 @@ -Drupal 7.xx, xxxx-xx-xx (development version) +Drupal 7.56, 2017-06-21 ----------------------- +- Fixed security issues (access bypass). See SA-CORE-2017-003. Drupal 7.55, 2017-06-07 ----------------------- diff --git a/includes/bootstrap.inc b/includes/bootstrap.inc index 4bc9df3c081..c06055edb15 100644 --- a/includes/bootstrap.inc +++ b/includes/bootstrap.inc @@ -8,7 +8,7 @@ /** * The current system version. */ -define('VERSION', '7.56-dev'); +define('VERSION', '7.56'); /** * Core API compatibility. diff --git a/includes/file.inc b/includes/file.inc index fafc33da53a..fa7f5eb547f 100644 --- a/includes/file.inc +++ b/includes/file.inc @@ -1614,6 +1614,20 @@ function file_save_upload($form_field_name, $validators = array(), $destination // If we made it this far it's safe to record this file in the database. if ($file = file_save($file)) { + // Track non-public files in the session if they were uploaded by an + // anonymous user. This allows modules such as the File module to only + // grant view access to the specific anonymous user who uploaded the file. + // See file_file_download(). + // The 'file_public_schema' variable is used to allow other publicly + // accessible file schemes to be treated the same as the public:// scheme + // provided by Drupal core and to avoid adding unnecessary data to the + // session (and the resulting bypass of the page cache) in those cases. For + // security reasons, only schemes that are completely publicly accessible, + // with no download restrictions, should be added to this variable. See + // file_managed_file_value(). + if (!$user->uid && !in_array($destination_scheme, variable_get('file_public_schema', array('public')))) { + $_SESSION['anonymous_allowed_file_ids'][$file->fid] = $file->fid; + } // Add file to the cache. $upload_cache[$form_field_name] = $file; return $file; diff --git a/modules/file/file.module b/modules/file/file.module index c8e17f85834..fb5e9b949d4 100644 --- a/modules/file/file.module +++ b/modules/file/file.module @@ -146,8 +146,9 @@ function file_file_download($uri, $field_type = 'file') { // headers for files controlled by other modules. Make an exception for // temporary files where the host entity has not yet been saved (for example, // an image preview on a node/add form) in which case, allow download by the - // file's owner. - if (empty($references) && ($file->status == FILE_STATUS_PERMANENT || $file->uid != $user->uid)) { + // file's owner. For anonymous file owners, only the browser session that + // uploaded the file should be granted access. + if (empty($references) && ($file->status == FILE_STATUS_PERMANENT || $file->uid != $user->uid || (!$user->uid && empty($_SESSION['anonymous_allowed_file_ids'][$file->fid])))) { return; } diff --git a/modules/file/tests/file.test b/modules/file/tests/file.test index 41e97cdfa34..b3a1424dc5c 100644 --- a/modules/file/tests/file.test +++ b/modules/file/tests/file.test @@ -1551,6 +1551,80 @@ class FilePrivateTestCase extends FileFieldTestCase { $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.'); + + // As an anonymous user, create a temporary file with no references and + // confirm that only the session that uploaded it may view it. + $this->drupalLogout(); + user_role_grant_permissions(DRUPAL_ANONYMOUS_RID, array( + "create $type_name content", + 'access content', + )); + $test_file = $this->getTestFile('text'); + $this->drupalGet('node/add/' . $type_name); + $edit = array('files[' . $field_name . '_' . LANGUAGE_NONE . '_0]' => drupal_realpath($test_file->uri)); + $this->drupalPost(NULL, $edit, t('Upload')); + $files = file_load_multiple(array(), array('uid' => 0)); + $this->assertEqual(1, count($files), 'Loaded one anonymous file.'); + $file = end($files); + $this->assertNotEqual($file->status, FILE_STATUS_PERMANENT, 'File is temporary.'); + $usage = file_usage_list($file); + $this->assertFalse($usage, 'No file usage found.'); + $file_url = file_create_url($file->uri); + $this->drupalGet($file_url); + $this->assertResponse(200, 'Confirmed that the anonymous uploader has access to the temporary file.'); + // Close the prior connection and remove the session cookie. + $this->curlClose(); + $this->cookies = array(); + $this->drupalGet($file_url); + $this->assertResponse(403, 'Confirmed that another anonymous user cannot access the temporary file.'); + + // As an anonymous user, create a permanent file that is referenced by a + // published node and confirm that all anonymous users may view it. + $test_file = $this->getTestFile('text'); + $this->drupalGet('node/add/' . $type_name); + $edit = array(); + $edit['title'] = $this->randomName(); + $edit['files[' . $field_name . '_' . LANGUAGE_NONE . '_0]'] = drupal_realpath($test_file->uri); + $this->drupalPost(NULL, $edit, t('Save')); + $new_node = $this->drupalGetNodeByTitle($edit['title']); + $file = file_load($new_node->{$field_name}[LANGUAGE_NONE][0]['fid']); + $this->assertEqual($file->status, FILE_STATUS_PERMANENT, 'File is permanent.'); + $usage = file_usage_list($file); + $this->assertTrue($usage, 'File usage found.'); + $file_url = file_create_url($file->uri); + $this->drupalGet($file_url); + $this->assertResponse(200, 'Confirmed that the anonymous uploader has access to the permanent file that is referenced by a published node.'); + // Close the prior connection and remove the session cookie. + $this->curlClose(); + $this->cookies = array(); + $this->drupalGet($file_url); + $this->assertResponse(200, 'Confirmed that another anonymous user also has access to the permanent file that is referenced by a published node.'); + + // As an anonymous user, create a permanent file that is referenced by an + // unpublished node and confirm that no anonymous users may view it (even + // the session that uploaded the file) because they cannot view the + // unpublished node. + $test_file = $this->getTestFile('text'); + $this->drupalGet('node/add/' . $type_name); + $edit = array(); + $edit['title'] = $this->randomName(); + $edit['files[' . $field_name . '_' . LANGUAGE_NONE . '_0]'] = drupal_realpath($test_file->uri); + $this->drupalPost(NULL, $edit, t('Save')); + $new_node = $this->drupalGetNodeByTitle($edit['title']); + $new_node->status = NODE_NOT_PUBLISHED; + node_save($new_node); + $file = file_load($new_node->{$field_name}[LANGUAGE_NONE][0]['fid']); + $this->assertEqual($file->status, FILE_STATUS_PERMANENT, 'File is permanent.'); + $usage = file_usage_list($file); + $this->assertTrue($usage, 'File usage found.'); + $file_url = file_create_url($file->uri); + $this->drupalGet($file_url); + $this->assertResponse(403, 'Confirmed that the anonymous uploader cannot access the permanent file when it is referenced by an unpublished node.'); + // Close the prior connection and remove the session cookie. + $this->curlClose(); + $this->cookies = array(); + $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.'); } }