diff options
author | David Rothstein <drothstein@gmail.com> | 2016-02-24 14:25:49 -0500 |
---|---|---|
committer | David Rothstein <drothstein@gmail.com> | 2016-02-24 14:25:49 -0500 |
commit | b8d9c44f83eca57039f648a0edb0f369f8d3e6b4 (patch) | |
tree | 2b97fdea5a364b7abc6728fb364244c0a52cbc73 /modules | |
parent | 2f54b101bf722849e456d859876b27b90ad7e479 (diff) | |
download | brdo-b8d9c44f83eca57039f648a0edb0f369f8d3e6b4.tar.gz brdo-b8d9c44f83eca57039f648a0edb0f369f8d3e6b4.tar.bz2 |
Revert "Drupal 7.43"
This reverts commit 2f54b101bf722849e456d859876b27b90ad7e479.
Diffstat (limited to 'modules')
-rw-r--r-- | modules/file/file.module | 15 | ||||
-rw-r--r-- | modules/file/tests/file.test | 138 | ||||
-rw-r--r-- | modules/simpletest/tests/common.test | 68 | ||||
-rw-r--r-- | modules/simpletest/tests/common_test.module | 9 | ||||
-rw-r--r-- | modules/simpletest/tests/xmlrpc.test | 34 | ||||
-rw-r--r-- | modules/system/system.admin.inc | 8 | ||||
-rw-r--r-- | modules/system/system.js | 2 | ||||
-rw-r--r-- | modules/system/system.test | 16 | ||||
-rw-r--r-- | modules/user/user.module | 16 |
9 files changed, 12 insertions, 294 deletions
diff --git a/modules/file/file.module b/modules/file/file.module index 9e091af03..fbf8b81ec 100644 --- a/modules/file/file.module +++ b/modules/file/file.module @@ -529,18 +529,13 @@ function file_managed_file_value(&$element, $input = FALSE, $form_state = NULL) // publicly accessible, with no download restrictions; for security // reasons all other schemes must go through the file_download_access() // check. - if (!in_array(file_uri_scheme($file->uri), variable_get('file_public_schema', array('public'))) && !file_download_access($file->uri)) { - $force_default = TRUE; - } - // Temporary files that belong to other users should never be allowed. - // Since file ownership can't be determined for anonymous users, they - // are not allowed to reuse temporary files at all. - elseif ($file->status != FILE_STATUS_PERMANENT && (!$GLOBALS['user']->uid || $file->uid != $GLOBALS['user']->uid)) { - $force_default = TRUE; + if (in_array(file_uri_scheme($file->uri), variable_get('file_public_schema', array('public'))) || file_download_access($file->uri)) { + $fid = $file->fid; } - // If all checks pass, allow the file to be changed. + // If the current user doesn't have access, don't let the file be + // changed. else { - $fid = $file->fid; + $force_default = TRUE; } } } diff --git a/modules/file/tests/file.test b/modules/file/tests/file.test index 6d7cb4bc4..80433954b 100644 --- a/modules/file/tests/file.test +++ b/modules/file/tests/file.test @@ -218,30 +218,6 @@ class FileFieldTestCase extends DrupalWebTestCase { $message = isset($message) ? $message : format_string('File %file is permanent.', array('%file' => $file->uri)); $this->assertTrue($file->status == FILE_STATUS_PERMANENT, $message); } - - /** - * Creates a temporary file, for a specific user. - * - * @param string $data - * A string containing the contents of the file. - * @param int $uid - * The user ID of the file owner. - * - * @return object - * A file object, or FALSE on error. - */ - function createTemporaryFile($data, $uid = NULL) { - $file = file_save_data($data, NULL, NULL); - - if ($file) { - $file->uid = isset($uid) ? $uid : $this->admin_user->uid; - // Change the file status to be temporary. - $file->status = NULL; - return file_save($file); - } - - return $file; - } } /** @@ -551,120 +527,6 @@ class FileFieldWidgetTestCase extends FileFieldTestCase { } /** - * Tests exploiting the temporary file removal of another user using fid. - */ - function testTemporaryFileRemovalExploit() { - // Create a victim user. - $victim_user = $this->drupalCreateUser(); - - // Create an attacker user. - $attacker_user = $this->drupalCreateUser(array( - 'access content', - 'create page content', - 'edit any page content', - )); - - // Log in as the attacker user. - $this->drupalLogin($attacker_user); - - // Perform tests using the newly created users. - $this->doTestTemporaryFileRemovalExploit($victim_user->uid, $attacker_user->uid); - } - - /** - * Tests exploiting the temporary file removal for anonymous users using fid. - */ - public function testTemporaryFileRemovalExploitAnonymous() { - // Set up an anonymous victim user. - $victim_uid = 0; - - // Set up an anonymous attacker user. - $attacker_uid = 0; - - // Set up permissions for anonymous attacker user. - user_role_change_permissions(DRUPAL_ANONYMOUS_RID, array( - 'access content' => TRUE, - 'create page content' => TRUE, - 'edit any page content' => TRUE, - )); - - // In order to simulate being the anonymous attacker user, we need to log - // out here since setUp() has logged in the admin. - $this->drupalLogout(); - - // Perform tests using the newly set up users. - $this->doTestTemporaryFileRemovalExploit($victim_uid, $attacker_uid); - } - - /** - * Helper for testing exploiting the temporary file removal using fid. - * - * @param int $victim_uid - * The victim user ID. - * @param int $attacker_uid - * The attacker user ID. - */ - protected function doTestTemporaryFileRemovalExploit($victim_uid, $attacker_uid) { - // Use 'page' instead of 'article', so that the 'article' image field does - // not conflict with this test. If in the future the 'page' type gets its - // own default file or image field, this test can be made more robust by - // using a custom node type. - $type_name = 'page'; - $field_name = 'test_file_field'; - $this->createFileField($field_name, $type_name); - - $test_file = $this->getTestFile('text'); - foreach (array('nojs', 'js') as $type) { - // Create a temporary file owned by the anonymous victim user. This will be - // as if they had uploaded the file, but not saved the node they were - // editing or creating. - $victim_tmp_file = $this->createTemporaryFile('some text', $victim_uid); - $victim_tmp_file = file_load($victim_tmp_file->fid); - $this->assertTrue($victim_tmp_file->status != FILE_STATUS_PERMANENT, 'New file saved to disk is temporary.'); - $this->assertFalse(empty($victim_tmp_file->fid), 'New file has a fid'); - $this->assertEqual($victim_uid, $victim_tmp_file->uid, 'New file belongs to the victim user'); - - // Have attacker create a new node with a different uploaded file and - // ensure it got uploaded successfully. - // @todo Can we test AJAX? See https://www.drupal.org/node/2538260 - $edit = array( - 'title' => $type . '-title', - ); - - // Attach a file to a node. - $langcode = LANGUAGE_NONE; - $edit['files[' . $field_name . '_' . $langcode . '_0]'] = drupal_realpath($test_file->uri); - $this->drupalPost("node/add/$type_name", $edit, 'Save'); - $node = $this->drupalGetNodeByTitle($edit['title']); - $node_file = file_load($node->{$field_name}[$langcode][0]['fid']); - $this->assertFileExists($node_file, 'New file saved to disk on node creation.'); - $this->assertEqual($attacker_uid, $node_file->uid, 'New file belongs to the attacker.'); - - // Ensure the file can be downloaded. - $this->drupalGet(file_create_url($node_file->uri)); - $this->assertResponse(200, 'Confirmed that the generated URL is correct by downloading the shipped file.'); - - // "Click" the remove button (emulating either a nojs or js submission). - // In this POST request, the attacker "guesses" the fid of the victim's - // temporary file and uses that to remove this file. - $this->drupalGet('node/' . $node->nid . '/edit'); - switch ($type) { - case 'nojs': - $this->drupalPost(NULL, array("{$field_name}[$langcode][0][fid]" => (string) $victim_tmp_file->fid), 'Remove'); - break; - case 'js': - $button = $this->xpath('//input[@type="submit" and @value="Remove"]'); - $this->drupalPostAJAX(NULL, array("{$field_name}[$langcode][0][fid]" => (string) $victim_tmp_file->fid), array((string) $button[0]['name'] => (string) $button[0]['value'])); - break; - } - - // The victim's temporary file should not be removed by the attacker's - // POST request. - $this->assertFileExists($victim_tmp_file); - } - } - - /** * Tests upload and remove buttons for multiple multi-valued File fields. */ function testMultiValuedWidget() { diff --git a/modules/simpletest/tests/common.test b/modules/simpletest/tests/common.test index 92aefe48f..bf8557619 100644 --- a/modules/simpletest/tests/common.test +++ b/modules/simpletest/tests/common.test @@ -373,65 +373,6 @@ class CommonURLUnitTest extends DrupalWebTestCase { } /** - * Tests url_is_external(). - */ -class UrlIsExternalUnitTest extends DrupalUnitTestCase { - - public static function getInfo() { - return array( - 'name' => 'External URL checking', - 'description' => 'Performs tests on url_is_external().', - 'group' => 'System', - ); - } - - /** - * Tests if each URL is external or not. - */ - function testUrlIsExternal() { - foreach ($this->examples() as $path => $expected) { - $this->assertIdentical(url_is_external($path), $expected, $path); - } - } - - /** - * Provides data for testUrlIsExternal(). - * - * @return array - * An array of test data, keyed by a path, with the expected value where - * TRUE is external, and FALSE is not external. - */ - protected function examples() { - return array( - // Simple external URLs. - 'http://example.com' => TRUE, - 'https://example.com' => TRUE, - 'http://drupal.org/foo/bar?foo=bar&bar=baz&baz#foo' => TRUE, - '//drupal.org' => TRUE, - // Some browsers ignore or strip leading control characters. - "\x00//www.example.com" => TRUE, - "\x08//www.example.com" => TRUE, - "\x1F//www.example.com" => TRUE, - "\n//www.example.com" => TRUE, - // JSON supports decoding directly from UTF-8 code points. - json_decode('"\u00AD"') . "//www.example.com" => TRUE, - json_decode('"\u200E"') . "//www.example.com" => TRUE, - json_decode('"\uE0020"') . "//www.example.com" => TRUE, - json_decode('"\uE000"') . "//www.example.com" => TRUE, - // Backslashes should be normalized to forward. - '\\\\example.com' => TRUE, - // Local URLs. - 'node' => FALSE, - '/system/ajax' => FALSE, - '?q=foo:bar' => FALSE, - 'node/edit:me' => FALSE, - '/drupal.org' => FALSE, - '<front>' => FALSE, - ); - } -} - -/** * Tests for check_plain(), filter_xss(), format_string(), and check_url(). */ class CommonXssUnitTest extends DrupalUnitTestCase { @@ -1315,15 +1256,6 @@ class DrupalGotoTest extends DrupalWebTestCase { $this->assertText('drupal_goto', 'Drupal goto redirect succeeded.'); $this->assertEqual($this->getUrl(), url('common-test/drupal_goto', array('query' => array('foo' => '123'), 'absolute' => TRUE)), 'Drupal goto redirected to expected URL.'); - // Test that calling drupal_goto() on the current path is not dangerous. - variable_set('common_test_redirect_current_path', TRUE); - $this->drupalGet('', array('query' => array('q' => 'http://www.example.com/'))); - $headers = $this->drupalGetHeaders(TRUE); - list(, $status) = explode(' ', $headers[0][':status'], 3); - $this->assertEqual($status, 302, 'Expected response code was sent.'); - $this->assertNotEqual($this->getUrl(), 'http://www.example.com/', 'Drupal goto did not redirect to external URL.'); - $this->assertTrue(strpos($this->getUrl(), url('<front>', array('absolute' => TRUE))) === 0, 'Drupal redirected to itself.'); - variable_del('common_test_redirect_current_path'); // Test that drupal_goto() respects ?destination=xxx. Use an complicated URL // to test that the path is encoded and decoded properly. $destination = 'common-test/drupal_goto/destination?foo=%2525&bar=123'; diff --git a/modules/simpletest/tests/common_test.module b/modules/simpletest/tests/common_test.module index 2eb8cd5d2..674a49446 100644 --- a/modules/simpletest/tests/common_test.module +++ b/modules/simpletest/tests/common_test.module @@ -93,15 +93,6 @@ function common_test_drupal_goto_alter(&$path, &$options, &$http_response_code) } /** - * Implements hook_init(). - */ -function common_test_init() { - if (variable_get('common_test_redirect_current_path', FALSE)) { - drupal_goto(current_path()); - } -} - -/** * Print destination query parameter. */ function common_test_destination() { diff --git a/modules/simpletest/tests/xmlrpc.test b/modules/simpletest/tests/xmlrpc.test index bb74f059b..1a9ef2349 100644 --- a/modules/simpletest/tests/xmlrpc.test +++ b/modules/simpletest/tests/xmlrpc.test @@ -246,38 +246,4 @@ class XMLRPCMessagesTestCase extends DrupalWebTestCase { $this->assertEqual($removed, 'system.methodSignature', 'Hiding builting system.methodSignature with hook_xmlrpc_alter works'); } - /** - * Test limits on system.multicall that can prevent brute-force attacks. - */ - function testMulticallLimit() { - $url = url(NULL, array('absolute' => TRUE)) . 'xmlrpc.php'; - $multicall_args = array(); - $num_method_calls = 10; - for ($i = 0; $i < $num_method_calls; $i++) { - $struct = array('i' => $i); - $multicall_args[] = array('methodName' => 'validator1.echoStructTest', 'params' => array($struct)); - } - // Test limits of 1, 5, 9, 13. - for ($limit = 1; $limit < $num_method_calls + 4; $limit += 4) { - variable_set('xmlrpc_multicall_duplicate_method_limit', $limit); - $results = xmlrpc($url, array('system.multicall' => array($multicall_args))); - $this->assertEqual($num_method_calls, count($results)); - for ($i = 0; $i < min($limit, $num_method_calls); $i++) { - $x = array_shift($results); - $this->assertTrue(empty($x->is_error), "Result $i is not an error"); - $this->assertEqual($multicall_args[$i]['params'][0], $x); - } - for (; $i < $num_method_calls; $i++) { - $x = array_shift($results); - $this->assertFalse(empty($x->is_error), "Result $i is an error"); - $this->assertEqual(-156579, $x->code); - } - } - variable_set('xmlrpc_multicall_duplicate_method_limit', -1); - $results = xmlrpc($url, array('system.multicall' => array($multicall_args))); - $this->assertEqual($num_method_calls, count($results)); - foreach ($results as $i => $x) { - $this->assertTrue(empty($x->is_error), "Result $i is not an error"); - } - } } diff --git a/modules/system/system.admin.inc b/modules/system/system.admin.inc index 16c40d4d4..0f525c6cf 100644 --- a/modules/system/system.admin.inc +++ b/modules/system/system.admin.inc @@ -2202,11 +2202,6 @@ function system_add_date_format_type_form_submit($form, &$form_state) { * Return the date for a given format string via Ajax. */ function system_date_time_lookup() { - // This callback is protected with a CSRF token because user input from the - // query string is reflected in the output. - if (!isset($_GET['token']) || !drupal_valid_token($_GET['token'], 'admin/config/regional/date-time/formats/lookup')) { - return MENU_ACCESS_DENIED; - } $result = format_date(REQUEST_TIME, 'custom', $_GET['format']); drupal_json_output($result); } @@ -2880,14 +2875,13 @@ function system_date_time_formats() { * Allow users to add additional date formats. */ function system_configure_date_formats_form($form, &$form_state, $dfid = 0) { - $ajax_path = 'admin/config/regional/date-time/formats/lookup'; $js_settings = array( 'type' => 'setting', 'data' => array( 'dateTime' => array( 'date-format' => array( 'text' => t('Displayed as'), - 'lookup' => url($ajax_path, array('query' => array('token' => drupal_get_token($ajax_path)))), + 'lookup' => url('admin/config/regional/date-time/formats/lookup'), ), ), ), diff --git a/modules/system/system.js b/modules/system/system.js index c0e76d38e..910fb5d3d 100644 --- a/modules/system/system.js +++ b/modules/system/system.js @@ -105,7 +105,7 @@ Drupal.behaviors.dateTime = { // Attach keyup handler to custom format inputs. $('input' + source, context).once('date-time').keyup(function () { var input = $(this); - var url = fieldSettings.lookup + (/\?/.test(fieldSettings.lookup) ? '&format=' : '?format=') + encodeURIComponent(input.val()); + var url = fieldSettings.lookup + (/\?q=/.test(fieldSettings.lookup) ? '&format=' : '?format=') + encodeURIComponent(input.val()); $.getJSON(url, function (data) { $(suffix).empty().append(' ' + fieldSettings.text + ': <em>' + data + '</em>'); }); diff --git a/modules/system/system.test b/modules/system/system.test index 95b43538b..bc764dde5 100644 --- a/modules/system/system.test +++ b/modules/system/system.test @@ -1350,23 +1350,7 @@ class DateTimeFunctionalTest extends DrupalWebTestCase { $this->assertEqual($this->getUrl(), url('admin/config/regional/date-time/formats', array('absolute' => TRUE)), 'Correct page redirection.'); $this->assertText(t('Custom date format updated.'), 'Custom date format successfully updated.'); - // Check that ajax callback is protected by CSRF token. - $this->drupalGet('admin/config/regional/date-time/formats/lookup', array('query' => array('format' => 'Y m d'))); - $this->assertResponse(403, 'Access denied with no token'); - $this->drupalGet('admin/config/regional/date-time/formats/lookup', array('query' => array('token' => 'invalid', 'format' => 'Y m d'))); - $this->assertResponse(403, 'Access denied with invalid token'); - $this->drupalGet('admin/config/regional/date-time/formats'); - $this->clickLink(t('edit')); - $settings = $this->drupalGetSettings(); - $lookup_url = $settings['dateTime']['date-format']['lookup']; - preg_match('/token=([^&]+)/', $lookup_url, $matches); - $this->assertFalse(empty($matches[1]), 'Found token value'); - $this->drupalGet('admin/config/regional/date-time/formats/lookup', array('query' => array('token' => $matches[1], 'format' => 'Y m d'))); - $this->assertResponse(200, 'Access allowed with valid token'); - $this->assertText(format_date(time(), 'custom', 'Y m d')); - // Delete custom date format. - $this->drupalGet('admin/config/regional/date-time/formats'); $this->clickLink(t('delete')); $this->drupalPost($this->getUrl(), array(), t('Remove')); $this->assertEqual($this->getUrl(), url('admin/config/regional/date-time/formats', array('absolute' => TRUE)), 'Correct page redirection.'); diff --git a/modules/user/user.module b/modules/user/user.module index d38de69b1..c33aa0982 100644 --- a/modules/user/user.module +++ b/modules/user/user.module @@ -1308,12 +1308,10 @@ function user_user_presave(&$edit, $account, $category) { elseif (!empty($edit['picture_delete'])) { $edit['picture'] = NULL; } - } - - // Filter out roles with empty values to avoid granting extra roles when - // processing custom form submissions. - if (isset($edit['roles'])) { - $edit['roles'] = array_filter($edit['roles']); + // Prepare user roles. + if (isset($edit['roles'])) { + $edit['roles'] = array_filter($edit['roles']); + } } // Move account cancellation information into $user->data. @@ -2229,11 +2227,7 @@ function user_login_final_validate($form, &$form_state) { } } else { - // Use $form_state['input']['name'] here to guarantee that we send - // exactly what the user typed in. $form_state['values']['name'] may have - // been modified by validation handlers that ran earlier than this one. - $query = isset($form_state['input']['name']) ? array('name' => $form_state['input']['name']) : array(); - form_set_error('name', t('Sorry, unrecognized username or password. <a href="@password">Have you forgotten your password?</a>', array('@password' => url('user/password', array('query' => $query))))); + form_set_error('name', t('Sorry, unrecognized username or password. <a href="@password">Have you forgotten your password?</a>', array('@password' => url('user/password', array('query' => array('name' => $form_state['values']['name'])))))); watchdog('user', 'Login attempt failed for %user.', array('%user' => $form_state['values']['name'])); } } |