diff --git a/core/CHANGELOG.txt b/core/CHANGELOG.txt
index 01a3dd484c166a75e6fef8dafe2d2d79f1c7d07f..0deaf904e2b16687a8dbb44e845cbd2beb1d295a 100644
--- a/core/CHANGELOG.txt
+++ b/core/CHANGELOG.txt
@@ -1,3 +1,7 @@
+Drupal 8.0.4, 2016-02-24
+------------------------
+- Fixed security issues (multiple vulnerabilities). See SA-CORE-2016-001.
+
Drupal 8.0.0, 2015-11-19
------------------------
- Significantly improved the front end:
diff --git a/core/lib/Drupal.php b/core/lib/Drupal.php
index 16231e9312763d24c401a11dbe56dc93ffb2ef6c..4c0883bbd84e6d5b945c2389dbf912e2bd049050 100644
--- a/core/lib/Drupal.php
+++ b/core/lib/Drupal.php
@@ -81,7 +81,7 @@ class Drupal {
/**
* The current system version.
*/
- const VERSION = '8.0.3';
+ const VERSION = '8.0.4';
/**
* Core API compatibility.
diff --git a/core/lib/Drupal/Component/Utility/UrlHelper.php b/core/lib/Drupal/Component/Utility/UrlHelper.php
index 9166b34a5edffe276975ff1de5a6201056d4b576..474a3e4d83184b0e498face3d69bbe85341e6f75 100644
--- a/core/lib/Drupal/Component/Utility/UrlHelper.php
+++ b/core/lib/Drupal/Component/Utility/UrlHelper.php
@@ -216,12 +216,19 @@ public static function encodePath($path) {
*/
public static function isExternal($path) {
$colonpos = strpos($path, ':');
- // Avoid calling drupal_strip_dangerous_protocols() if there is any slash
- // (/), hash (#) or question_mark (?) before the colon (:) occurrence - if
- // any - as this would clearly mean it is not a URL. If the path starts with
- // 2 slashes then it is always considered an external URL without an
- // explicit protocol part.
+ // Some browsers treat \ as / so normalize to forward slashes.
+ $path = str_replace('\\', '/', $path);
+ // If the path starts with 2 slashes then it is always considered an
+ // external URL without an explicit protocol part.
return (strpos($path, '//') === 0)
+ // Leading control characters may be ignored or mishandled by browsers,
+ // so assume such a path may lead to an external location. The \p{C}
+ // character class matches all UTF-8 control, unassigned, and private
+ // characters.
+ || (preg_match('/^\p{C}/u', $path) !== 0)
+ // Avoid calling static::stripDangerousProtocols() if there is any slash
+ // (/), hash (#) or question_mark (?) before the colon (:) occurrence -
+ // if any - as this would clearly mean it is not a URL.
|| ($colonpos !== FALSE
&& !preg_match('![/?#]!', substr($path, 0, $colonpos))
&& static::stripDangerousProtocols($path) == $path);
diff --git a/core/modules/file/src/Element/ManagedFile.php b/core/modules/file/src/Element/ManagedFile.php
index 1d9a742cebb0c545c017e67f0fcf1c92716b906a..df5c28b8c15fbe62f951d69866bc8ab9862e3dbe 100644
--- a/core/modules/file/src/Element/ManagedFile.php
+++ b/core/modules/file/src/Element/ManagedFile.php
@@ -64,6 +64,7 @@ public static function valueCallback(&$element, $input, FormStateInterface $form
foreach ($fids as $key => $fid) {
$fids[$key] = (int) $fid;
}
+ $force_default = FALSE;
// Process any input and save new uploads.
if ($input !== FALSE) {
@@ -95,15 +96,26 @@ public static function valueCallback(&$element, $input, FormStateInterface $form
$fids = [];
foreach ($input['fids'] as $fid) {
if ($file = File::load($fid)) {
- $fids[] = $file->id();
+ // 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.
+ if ($file->isTemporary() && (\Drupal::currentUser()->isAnonymous() || $file->getOwnerId() != \Drupal::currentUser()->id())) {
+ $force_default = TRUE;
+ break;
+ }
+ // If all checks pass, allow the files to be changed.
+ else {
+ $fids[] = $file->id();
+ }
}
}
}
}
}
- // If there is no input, set the default value.
- else {
+ // If there is no input or if the default value was requested above, use the
+ // default value.
+ if ($input === FALSE || $force_default) {
if ($element['#extended']) {
$default_fids = isset($element['#default_value']['fids']) ? $element['#default_value']['fids'] : [];
$return = isset($element['#default_value']) ? $element['#default_value'] : ['fids' => []];
diff --git a/core/modules/file/src/Tests/FileFieldWidgetTest.php b/core/modules/file/src/Tests/FileFieldWidgetTest.php
index 7b0cce5455237725ba261e1979c56729779ca95e..1fa40ce446e4679f2da330ee03368ea15985f4fc 100644
--- a/core/modules/file/src/Tests/FileFieldWidgetTest.php
+++ b/core/modules/file/src/Tests/FileFieldWidgetTest.php
@@ -10,11 +10,14 @@
use Drupal\comment\Entity\Comment;
use Drupal\comment\Tests\CommentTestTrait;
use Drupal\Component\Utility\Unicode;
+use Drupal\Core\Url;
use Drupal\field\Entity\FieldConfig;
use Drupal\field\Entity\FieldStorageConfig;
use Drupal\field_ui\Tests\FieldUiTestTrait;
use Drupal\user\RoleInterface;
use Drupal\file\Entity\File;
+use Drupal\user\Entity\User;
+use Drupal\user\UserInterface;
/**
* Tests the file field widget, single and multi-valued, with and without AJAX,
@@ -42,6 +45,36 @@ protected function setUp() {
*/
public static $modules = array('comment', 'block');
+ /**
+ * Creates a temporary file, for a specific user.
+ *
+ * @param string $data
+ * A string containing the contents of the file.
+ * @param \Drupal\user\UserInterface $user
+ * The user of the file owner.
+ *
+ * @return \Drupal\file\FileInterface
+ * A file object, or FALSE on error.
+ */
+ protected function createTemporaryFile($data, UserInterface $user = NULL) {
+ $file = file_save_data($data, NULL, NULL);
+
+ if ($file) {
+ if ($user) {
+ $file->setOwner($user);
+ }
+ else {
+ $file->setOwner($this->adminUser);
+ }
+ // Change the file status to be temporary.
+ $file->setTemporary();
+ // Save the changes.
+ $file->save();
+ }
+
+ return $file;
+ }
+
/**
* Tests upload and remove buttons for a single-valued File field.
*/
@@ -449,4 +482,113 @@ public function testWidgetElement() {
$this->assertIdentical(count($elements), 1);
}
+ /**
+ * Tests exploiting the temporary file removal of another user using fid.
+ */
+ public function testTemporaryFileRemovalExploit() {
+ // Create a victim user.
+ $victim_user = $this->drupalCreateUser();
+
+ // Create an attacker user.
+ $attacker_user = $this->drupalCreateUser(array(
+ 'access content',
+ 'create article content',
+ 'edit any article content',
+ ));
+
+ // Log in as the attacker user.
+ $this->drupalLogin($attacker_user);
+
+ // Perform tests using the newly created users.
+ $this->doTestTemporaryFileRemovalExploit($victim_user, $attacker_user);
+ }
+
+ /**
+ * Tests exploiting the temporary file removal for anonymous users using fid.
+ */
+ public function testTemporaryFileRemovalExploitAnonymous() {
+ // Set up an anonymous victim user.
+ $victim_user = User::getAnonymousUser();
+
+ // Set up an anonymous attacker user.
+ $attacker_user = User::getAnonymousUser();
+
+ // Set up permissions for anonymous attacker user.
+ user_role_change_permissions(RoleInterface::ANONYMOUS_ID, array(
+ 'access content' => TRUE,
+ 'create article content' => TRUE,
+ 'edit any article content' => TRUE,
+ ));
+
+ // Log out so as to be the anonymous attacker user.
+ $this->drupalLogout();
+
+ // Perform tests using the newly set up anonymous users.
+ $this->doTestTemporaryFileRemovalExploit($victim_user, $attacker_user);
+ }
+
+ /**
+ * Helper for testing exploiting the temporary file removal using fid.
+ *
+ * @param \Drupal\user\UserInterface $victim_user
+ * The victim user.
+ * @param \Drupal\user\UserInterface $attacker_user
+ * The attacker user.
+ */
+ protected function doTestTemporaryFileRemovalExploit(UserInterface $victim_user, UserInterface $attacker_user) {
+ $type_name = 'article';
+ $field_name = 'test_file_field';
+ $this->createFileField($field_name, 'node', $type_name);
+
+ $test_file = $this->getTestFile('text');
+ foreach (array('nojs', 'js') as $type) {
+ // Create a temporary file owned by the 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_user);
+ $victim_tmp_file = File::load($victim_tmp_file->id());
+ $this->assertTrue($victim_tmp_file->isTemporary(), 'New file saved to disk is temporary.');
+ $this->assertFalse(empty($victim_tmp_file->id()), 'New file has an fid.');
+ $this->assertEqual($victim_user->id(), $victim_tmp_file->getOwnerId(), 'New file belongs to the victim.');
+
+ // Have attacker create a new node with a different uploaded file and
+ // ensure it got uploaded successfully.
+ $edit = [
+ 'title[0][value]' => $type . '-title' ,
+ ];
+
+ // Attach a file to a node.
+ $edit['files[' . $field_name . '_0]'] = $this->container->get('file_system')->realpath($test_file->getFileUri());
+ $this->drupalPostForm(Url::fromRoute('node.add', array('node_type' => $type_name)), $edit, t('Save'));
+ $node = $this->drupalGetNodeByTitle($edit['title[0][value]']);
+
+ /** @var \Drupal\file\FileInterface $node_file */
+ $node_file = File::load($node->{$field_name}->target_id);
+ $this->assertFileExists($node_file, 'A file was saved to disk on node creation');
+ $this->assertEqual($attacker_user->id(), $node_file->getOwnerId(), 'New file belongs to the attacker.');
+
+ // Ensure the file can be downloaded.
+ $this->drupalGet(file_create_url($node_file->getFileUri()));
+ $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->toUrl('edit-form'));
+ switch ($type) {
+ case 'nojs':
+ $this->drupalPostForm(NULL, [$field_name . '[0][fids]' => (string) $victim_tmp_file->id()], 'Remove');
+ break;
+
+ case 'js':
+ $this->drupalPostAjaxForm(NULL, [$field_name . '[0][fids]' => (string) $victim_tmp_file->id()], ["{$field_name}_0_remove_button" => 'Remove']);
+ break;
+ }
+
+ // The victim's temporary file should not be removed by the attacker's
+ // POST request.
+ $this->assertFileExists($victim_tmp_file);
+ }
+ }
+
}
diff --git a/core/modules/user/src/Form/UserLoginForm.php b/core/modules/user/src/Form/UserLoginForm.php
index b3360910a743913c5516082f43f6c9c1cf66ab9b..28596eb289f4d76996b2b7fd88d95cdfaaa5d135 100644
--- a/core/modules/user/src/Form/UserLoginForm.php
+++ b/core/modules/user/src/Form/UserLoginForm.php
@@ -231,7 +231,13 @@ public function validateFinal(array &$form, FormStateInterface $form_state) {
}
}
else {
- $form_state->setErrorByName('name', $this->t('Unrecognized username or password. Have you forgotten your password?', array(':password' => $this->url('user.pass', [], array('query' => array('name' => $form_state->getValue('name')))))));
+ // Use $form_state->getUserInput() in the error message to guarantee
+ // that we send exactly what the user typed in. The value from
+ // $form_state->getValue() may have been modified by validation
+ // handlers that ran earlier than this one.
+ $user_input = $form_state->getUserInput();
+ $query = isset($user_input['name']) ? array('name' => $user_input['name']) : array();
+ $form_state->setErrorByName('name', $this->t('Unrecognized username or password. Have you forgotten your password?', array(':password' => $this->url('user.pass', [], array('query' => $query)))));
$accounts = $this->userStorage->loadByProperties(array('name' => $form_state->getValue('name')));
if (!empty($accounts)) {
$this->logger('user')->notice('Login attempt failed for %user.', array('%user' => $form_state->getValue('name')));
diff --git a/core/tests/Drupal/Tests/Component/Utility/UrlHelperTest.php b/core/tests/Drupal/Tests/Component/Utility/UrlHelperTest.php
index 761fa6960f197a7df294fa09b7b65d0bad0d0474..e2c85053e2670d618779ace490a8570501b903e6 100644
--- a/core/tests/Drupal/Tests/Component/Utility/UrlHelperTest.php
+++ b/core/tests/Drupal/Tests/Component/Utility/UrlHelperTest.php
@@ -361,6 +361,30 @@ public static function providerTestIsExternal() {
array('//www.drupal.org/foo/bar?foo=bar&bar=baz&baz#foo', TRUE),
// Internal URL starting with a slash.
array('/www.drupal.org', FALSE),
+ // Simple external URLs.
+ array('http://example.com', TRUE),
+ array('https://example.com', TRUE),
+ array('http://drupal.org/foo/bar?foo=bar&bar=baz&baz#foo', TRUE),
+ array('//drupal.org', TRUE),
+ // Some browsers ignore or strip leading control characters.
+ array("\x00//www.example.com", TRUE),
+ array("\x08//www.example.com", TRUE),
+ array("\x1F//www.example.com", TRUE),
+ array("\n//www.example.com", TRUE),
+ // JSON supports decoding directly from UTF-8 code points.
+ array(json_decode('"\u00AD"') . "//www.example.com", TRUE),
+ array(json_decode('"\u200E"') . "//www.example.com", TRUE),
+ array(json_decode('"\uE0020"') . "//www.example.com", TRUE),
+ array(json_decode('"\uE000"') . "//www.example.com", TRUE),
+ // Backslashes should be normalized to forward.
+ array('\\\\example.com', TRUE),
+ // Local URLs.
+ array('node', FALSE),
+ array('/system/ajax', FALSE),
+ array('?q=foo:bar', FALSE),
+ array('node/edit:me', FALSE),
+ array('/drupal.org', FALSE),
+ array('', FALSE),
);
}