From 4858e8e8dec13f544fba4970ea5ee301fcffba62 Mon Sep 17 00:00:00 2001 From: Chris McCafferty <cilefen@gmail.com> Date: Wed, 15 Mar 2017 16:49:00 -0400 Subject: [PATCH] SA-CORE-2017-001 by alexpott, benjy, Berdir, casey, cilefen, dawehner, Heine, klausi, larowlan, Mixologic, mlhess, pwolanin, samuel.mortenson, Wim Leers, xjm, YesCT --- composer.lock | 10 ++--- core/composer.json | 2 +- core/modules/block/block.routing.yml | 2 + core/modules/block/src/Tests/BlockUiTest.php | 14 +++++++ core/modules/editor/editor.module | 4 +- .../EditorPrivateFileReferenceFilterTest.php | 42 +++++++++++++++++-- core/modules/search/search.routing.yml | 3 ++ .../search/src/Form/SearchBlockForm.php | 13 ++++-- .../Tests/SearchConfigSettingsFormTest.php | 36 ++++++++++++++-- core/modules/simpletest/src/WebTestBase.php | 4 +- 10 files changed, 109 insertions(+), 21 deletions(-) diff --git a/composer.lock b/composer.lock index 9383a8f325fe..1d36a2335993 100644 --- a/composer.lock +++ b/composer.lock @@ -3642,16 +3642,16 @@ }, { "name": "phpunit/phpunit", - "version": "4.8.27", + "version": "4.8.28", "source": { "type": "git", "url": "https://github.com/sebastianbergmann/phpunit.git", - "reference": "c062dddcb68e44b563f66ee319ddae2b5a322a90" + "reference": "558a3a0d28b4cb7e4a593a4fbd2220e787076225" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/sebastianbergmann/phpunit/zipball/c062dddcb68e44b563f66ee319ddae2b5a322a90", - "reference": "c062dddcb68e44b563f66ee319ddae2b5a322a90", + "url": "https://api.github.com/repos/sebastianbergmann/phpunit/zipball/558a3a0d28b4cb7e4a593a4fbd2220e787076225", + "reference": "558a3a0d28b4cb7e4a593a4fbd2220e787076225", "shasum": "" }, "require": { @@ -3710,7 +3710,7 @@ "testing", "xunit" ], - "time": "2016-07-21T06:48:14+00:00" + "time": "2016-11-14T06:25:28+00:00" }, { "name": "phpunit/phpunit-mock-objects", diff --git a/core/composer.json b/core/composer.json index 503704cb7d2a..d2a41c0ae35d 100644 --- a/core/composer.json +++ b/core/composer.json @@ -42,7 +42,7 @@ "jcalderonzumba/gastonjs": "~1.0.2", "jcalderonzumba/mink-phantomjs-driver": "~0.3.1", "mikey179/vfsStream": "~1.2", - "phpunit/phpunit": "~4.8", + "phpunit/phpunit": ">=4.8.28 <5", "symfony/css-selector": "~2.8" }, "replace": { diff --git a/core/modules/block/block.routing.yml b/core/modules/block/block.routing.yml index e12cd8b1af82..83648b948303 100644 --- a/core/modules/block/block.routing.yml +++ b/core/modules/block/block.routing.yml @@ -32,6 +32,7 @@ entity.block.enable: op: enable requirements: _entity_access: 'block.enable' + _csrf_token: 'TRUE' entity.block.disable: path: '/admin/structure/block/manage/{block}/disable' @@ -40,6 +41,7 @@ entity.block.disable: op: disable requirements: _entity_access: 'block.disable' + _csrf_token: 'TRUE' block.admin_display: path: '/admin/structure/block' diff --git a/core/modules/block/src/Tests/BlockUiTest.php b/core/modules/block/src/Tests/BlockUiTest.php index 899f9bba8761..d4ebda4d9b7c 100644 --- a/core/modules/block/src/Tests/BlockUiTest.php +++ b/core/modules/block/src/Tests/BlockUiTest.php @@ -312,4 +312,18 @@ public function testBlockValidateErrors() { $this->assertTrue($error_class, 'Plugin error class found in parent form.'); } + /** + * Tests that the enable/disable routes are protected from CSRF. + */ + public function testRouteProtection() { + // Get the first block generated in our setUp method. + /** @var \Drupal\block\BlockInterface $block */ + $block = reset($this->blocks); + // Ensure that the enable and disable routes are protected. + $this->drupalGet('admin/structure/block/manage/' . $block->id() . '/disable'); + $this->assertResponse(403); + $this->drupalGet('admin/structure/block/manage/' . $block->id() . '/enable'); + $this->assertResponse(403); + } + } diff --git a/core/modules/editor/editor.module b/core/modules/editor/editor.module index 04e4cd5a3c13..dccf8239b5f0 100644 --- a/core/modules/editor/editor.module +++ b/core/modules/editor/editor.module @@ -520,8 +520,8 @@ function editor_file_download($uri) { if ($file->isPermanent()) { $referencing_entity_is_accessible = FALSE; $references = empty($usage_list['editor']) ? [] : $usage_list['editor']; - foreach ($references as $entity_type => $entity_ids) { - $referencing_entities = entity_load_multiple($entity_type, $entity_ids); + foreach ($references as $entity_type => $entity_ids_usage_count) { + $referencing_entities = entity_load_multiple($entity_type, array_keys($entity_ids_usage_count)); /** @var \Drupal\Core\Entity\EntityInterface $referencing_entity */ foreach ($referencing_entities as $referencing_entity) { if ($referencing_entity->access('view', NULL, TRUE)->isAllowed()) { diff --git a/core/modules/editor/tests/src/Functional/EditorPrivateFileReferenceFilterTest.php b/core/modules/editor/tests/src/Functional/EditorPrivateFileReferenceFilterTest.php index 3e3f3d187c2e..816de5d6e4f7 100644 --- a/core/modules/editor/tests/src/Functional/EditorPrivateFileReferenceFilterTest.php +++ b/core/modules/editor/tests/src/Functional/EditorPrivateFileReferenceFilterTest.php @@ -68,9 +68,18 @@ public function testEditorPrivateFileReferenceFilter() { $file->setPermanent(); $file->save(); + // Create some nodes to ensure file usage count does not match the ID's + // of the nodes we are going to check. + for ($i = 0; $i < 5; $i++) { + $this->drupalCreateNode([ + 'type' => 'page', + 'uid' => $author->id(), + ]); + } + // Create a node with its body field properly pointing to the just-created // file. - $node = $this->drupalCreateNode([ + $published_node = $this->drupalCreateNode([ 'type' => 'page', 'body' => [ 'value' => '<img alt="alt" data-entity-type="file" data-entity-uuid="' . $file->uuid() . '" src="' . $src . '" />', @@ -79,19 +88,44 @@ public function testEditorPrivateFileReferenceFilter() { 'uid' => $author->id(), ]); + // Create an unpublished node with its body field properly pointing to the + // just-created file. + $unpublished_node = $this->drupalCreateNode([ + 'type' => 'page', + 'status' => NODE_NOT_PUBLISHED, + 'body' => [ + 'value' => '<img alt="alt" data-entity-type="file" data-entity-uuid="' . $file->uuid() . '" src="' . $src . '" />', + 'format' => 'private_images', + ], + 'uid' => $author->id(), + ]); + // Do the actual test. The image should be visible for anonymous users, - // because they can view the referencing entity. - $this->drupalGet($node->toUrl()); + // because they can view the published node. Even though they can't view + // the unpublished node. + $this->drupalGet($published_node->toUrl()); $this->assertSession()->statusCodeEquals(200); + $this->drupalGet($unpublished_node->toUrl()); + $this->assertSession()->statusCodeEquals(403); $this->drupalGet($src); $this->assertSession()->statusCodeEquals(200); + // When the published node is also unpublished, the image should also + // become inaccessible to anonymous users. + $published_node->setPublished(FALSE)->save(); + + $this->drupalGet($published_node->toUrl()); + $this->assertSession()->statusCodeEquals(403); + $this->drupalGet($src); + $this->assertSession()->statusCodeEquals(403); + // Disallow anonymous users to view the entity, which then should also // disallow them to view the image. + $published_node->setPublished(TRUE)->save(); Role::load(RoleInterface::ANONYMOUS_ID) ->revokePermission('access content') ->save(); - $this->drupalGet($node->toUrl()); + $this->drupalGet($published_node->toUrl()); $this->assertSession()->statusCodeEquals(403); $this->drupalGet($src); $this->assertSession()->statusCodeEquals(403); diff --git a/core/modules/search/search.routing.yml b/core/modules/search/search.routing.yml index ec273f166882..98969a6e21c7 100644 --- a/core/modules/search/search.routing.yml +++ b/core/modules/search/search.routing.yml @@ -37,6 +37,7 @@ entity.search_page.enable: op: 'enable' requirements: _entity_access: 'search_page.update' + _csrf_token: 'TRUE' entity.search_page.disable: path: '/admin/config/search/pages/manage/{search_page}/disable' @@ -45,6 +46,7 @@ entity.search_page.disable: op: 'disable' requirements: _entity_access: 'search_page.disable' + _csrf_token: 'TRUE' entity.search_page.set_default: path: '/admin/config/search/pages/manage/{search_page}/set-default' @@ -52,6 +54,7 @@ entity.search_page.set_default: _controller: '\Drupal\search\Controller\SearchController::setAsDefault' requirements: _entity_access: 'search_page.update' + _csrf_token: 'TRUE' entity.search_page.delete_form: path: '/admin/config/search/pages/manage/{search_page}/delete' diff --git a/core/modules/search/src/Form/SearchBlockForm.php b/core/modules/search/src/Form/SearchBlockForm.php index 5d77b7c3bfb2..652bf732d12f 100644 --- a/core/modules/search/src/Form/SearchBlockForm.php +++ b/core/modules/search/src/Form/SearchBlockForm.php @@ -75,6 +75,16 @@ public function getFormId() { public function buildForm(array $form, FormStateInterface $form_state) { // Set up the form to submit using GET to the correct search page. $entity_id = $this->searchPageRepository->getDefaultSearchPage(); + + $form = []; + + // SearchPageRepository::getDefaultSearchPage() depends on search.settings. + // The dependency needs to be added before the conditional return, otherwise + // the block would get cached without the necessary cacheablity metadata in + // case there is no default search page and would not be invalidated if that + // changes. + $this->renderer->addCacheableDependency($form, $this->configFactory->get('search.settings')); + if (!$entity_id) { $form['message'] = [ '#markup' => $this->t('Search is currently disabled'), @@ -103,9 +113,6 @@ public function buildForm(array $form, FormStateInterface $form_state) { '#name' => '', ]; - // SearchPageRepository::getDefaultSearchPage() depends on search.settings. - $this->renderer->addCacheableDependency($form, $this->configFactory->get('search.settings')); - return $form; } diff --git a/core/modules/search/src/Tests/SearchConfigSettingsFormTest.php b/core/modules/search/src/Tests/SearchConfigSettingsFormTest.php index b5021d9ec5cb..191e0cf0d577 100644 --- a/core/modules/search/src/Tests/SearchConfigSettingsFormTest.php +++ b/core/modules/search/src/Tests/SearchConfigSettingsFormTest.php @@ -154,8 +154,7 @@ public function testSearchModuleDisabling() { // Test each plugin if it's enabled as the only search plugin. foreach ($entities as $entity_id => $entity) { - // Set this as default. - $this->drupalGet("admin/config/search/pages/manage/$entity_id/set-default"); + $this->setDefaultThroughUi($entity_id); // Run a search from the correct search URL. $info = $plugin_info[$entity_id]; @@ -187,13 +186,16 @@ public function testSearchModuleDisabling() { $entity->disable()->save(); } + // Set the node search as default. + $this->setDefaultThroughUi('node_search'); + // Test with all search plugins enabled. When you go to the search // page or run search, all plugins should be shown. foreach ($entities as $entity) { $entity->enable()->save(); } - // Set the node search as default. - $this->drupalGet('admin/config/search/pages/manage/node_search/set-default'); + + \Drupal::service('router.builder')->rebuild(); $paths = [ ['path' => 'search/node', 'options' => ['query' => ['keys' => 'pizza']]], @@ -316,6 +318,19 @@ public function testMultipleSearchPages() { $this->verifySearchPageOperations($first_id, FALSE, FALSE, FALSE, FALSE); } + /** + * Tests that the enable/disable/default routes are protected from CSRF. + */ + public function testRouteProtection() { + // Ensure that the enable and disable routes are protected. + $this->drupalGet('admin/config/search/pages/manage/node_search/enable'); + $this->assertResponse(403); + $this->drupalGet('admin/config/search/pages/manage/node_search/disable'); + $this->assertResponse(403); + $this->drupalGet('admin/config/search/pages/manage/node_search/set-default'); + $this->assertResponse(403); + } + /** * Checks that the search page operations match expectations. * @@ -373,4 +388,17 @@ protected function assertDefaultSearch($expected, $message = '', $group = 'Other $this->assertIdentical($search_page_repository->getDefaultSearchPage(), $expected, $message, $group); } + /** + * Sets a search page as the default in the UI. + * + * @param string $entity_id + * The search page entity ID to enable. + */ + protected function setDefaultThroughUi($entity_id) { + $this->drupalGet('admin/config/search/pages'); + preg_match('|href="([^"]+' . $entity_id . '/set-default[^"]+)"|', $this->getRawContent(), $matches); + + $this->drupalGet($this->getAbsoluteUrl($matches[1])); + } + } diff --git a/core/modules/simpletest/src/WebTestBase.php b/core/modules/simpletest/src/WebTestBase.php index 986410d19799..2491bc84b30f 100644 --- a/core/modules/simpletest/src/WebTestBase.php +++ b/core/modules/simpletest/src/WebTestBase.php @@ -901,7 +901,7 @@ protected function drupalGet($path, array $options = [], array $headers = []) { } if ($path instanceof Url) { - $path = $path->toString(); + $path = $path->setAbsolute()->toString(TRUE)->getGeneratedUrl(); } $verbose = 'GET request to: ' . $path . @@ -2127,7 +2127,7 @@ protected function buildUrl($path, array $options = []) { $url_options = $path->getOptions(); $options = $url_options + $options; $path->setOptions($options); - return $path->setAbsolute()->toString(); + return $path->setAbsolute()->toString(TRUE)->getGeneratedUrl(); } // The URL generator service is not necessarily available yet; e.g., in // interactive installer tests. -- GitLab