SA-CORE-2016-005 by larowlan, xjm, David_Rothstein, Dave Reid, Crell, cilefen,...

SA-CORE-2016-005 by larowlan, xjm, David_Rothstein, Dave Reid, Crell, cilefen, alexpott, mlhess, catch, pwolanin, YesCT, dawehner, quicksketch, Heine, znerol, charlotte.b, jnicola, ezraw

core/lib/Drupal/Core/Database/Query/Select.php

@@ -5,7 +5,6 @@
 use Drupal\Core\Database\Database;
 use Drupal\Core\Database\Connection;
 
-
 /**
  * Query builder for SELECT statements.
  *
@@ -456,6 +455,22 @@ public function preExecute(SelectInterface $query = NULL) {
 
     // Modules may alter all queries or only those having a particular tag.
     if (isset($this->alterTags)) {
+      // Many contrib modules as well as Entity Reference in core assume that
+      // query tags used for access-checking purposes follow the pattern
+      // $entity_type . '_access'. But this is not the case for taxonomy terms,
+      // since the core Taxonomy module used to add term_access instead of
+      // taxonomy_term_access to its queries. Provide backwards compatibility
+      // by adding both tags here instead of attempting to fix all contrib
+      // modules in a coordinated effort.
+      // TODO:
+      // - Extract this mechanism into a hook as part of a public (non-security)
+      //   issue.
+      // - Emit E_USER_DEPRECATED if term_access is used.
+      //   https://www.drupal.org/node/2575081
+      $term_access_tags = array('term_access' => 1, 'taxonomy_term_access' => 1);
+      if (array_intersect_key($this->alterTags, $term_access_tags)) {
+        $this->alterTags += $term_access_tags;
+      }
       $hooks = array('query');
       foreach ($this->alterTags as $tag => $value) {
         $hooks[] = 'query_' . $tag;

core/lib/Drupal/Core/Render/Element/MachineName.php

@@ -190,6 +190,7 @@ public static function processMachineName(&$element, FormStateInterface $form_st
     $element['#attached']['library'][] = 'core/drupal.machine-name';
     $options = [
       'replace_pattern',
+      'replace_token',
       'replace',
       'maxlength',
       'target',
@@ -198,6 +199,11 @@ public static function processMachineName(&$element, FormStateInterface $form_st
       'field_suffix',
       'suffix',
     ];
+
+    /** @var \Drupal\Core\Access\CsrfTokenGenerator $token_generator */
+    $token_generator = \Drupal::service('csrf_token');
+    $element['#machine_name']['replace_token'] = $token_generator->get($element['#machine_name']['replace_pattern']);
+
     $element['#attached']['drupalSettings']['machineName']['#' . $source['#id']] = array_intersect_key($element['#machine_name'], array_flip($options));
     $element['#attached']['drupalSettings']['langcode'] = $language->getId();
 

core/misc/machine-name.js

@@ -186,6 +186,8 @@
      * @param {string} settings.replace_pattern
      *   A regular expression (without modifiers) matching disallowed characters
      *   in the machine name; e.g., '[^a-z0-9]+'.
+     * @param {string} settings.replace_token
+     *   A token to validate the regular expression.
      * @param {string} settings.replace
      *   A character to replace disallowed characters with; e.g., '_' or '-'.
      * @param {number} settings.maxlength
@@ -199,6 +201,7 @@
         text: source,
         langcode: drupalSettings.langcode,
         replace_pattern: settings.replace_pattern,
+        replace_token: settings.replace_token,
         replace: settings.replace,
         lowercase: true
       });

core/modules/system/src/MachineNameController.php

@@ -4,7 +4,9 @@
 
 use Drupal\Component\Transliteration\TransliterationInterface;
 use Drupal\Component\Utility\Unicode;
+use Drupal\Core\Access\CsrfTokenGenerator;
 use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
+use Symfony\Component\HttpFoundation\File\Exception\AccessDeniedException;
 use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\DependencyInjection\ContainerInterface;
@@ -21,14 +23,24 @@ class MachineNameController implements ContainerInjectionInterface {
    */
   protected $transliteration;
 
+  /**
+   * The token generator.
+   *
+   * @var \Drupal\Core\Access\CsrfTokenGenerator
+   */
+  protected $tokenGenerator;
+
   /**
    * Constructs a MachineNameController object.
    *
    * @param \Drupal\Component\Transliteration\TransliterationInterface $transliteration
    *   The transliteration helper.
+   * @param \Drupal\Core\Access\CsrfTokenGenerator $token_generator
+   *   The token generator.
    */
-  public function __construct(TransliterationInterface $transliteration) {
+  public function __construct(TransliterationInterface $transliteration, CsrfTokenGenerator $token_generator) {
     $this->transliteration = $transliteration;
+    $this->tokenGenerator = $token_generator;
   }
 
   /**
@@ -36,7 +48,8 @@ public function __construct(TransliterationInterface $transliteration) {
    */
   public static function create(ContainerInterface $container) {
     return new static(
-      $container->get('transliteration')
+      $container->get('transliteration'),
+      $container->get('csrf_token')
     );
   }
 
@@ -54,6 +67,7 @@ public function transliterate(Request $request) {
     $text = $request->query->get('text');
     $langcode = $request->query->get('langcode');
     $replace_pattern = $request->query->get('replace_pattern');
+    $replace_token = $request->query->get('replace_token');
     $replace = $request->query->get('replace');
     $lowercase = $request->query->get('lowercase');
 
@@ -61,7 +75,15 @@ public function transliterate(Request $request) {
     if ($lowercase) {
       $transliterated = Unicode::strtolower($transliterated);
     }
+
     if (isset($replace_pattern) && isset($replace)) {
+      if (!isset($replace_token)) {
+        throw new AccessDeniedException("Missing 'replace_token' query parameter.");
+      }
+      elseif (!$this->tokenGenerator->validate($replace_token, $replace_pattern)) {
+        throw new AccessDeniedException("Invalid 'replace_token' query parameter.");
+      }
+
       // Quote the pattern delimiter and remove null characters to avoid the e
       // or other modifiers being injected.
       $transliterated = preg_replace('@' . strtr($replace_pattern, ['@' => '\@', chr(0) => '']) . '@', $replace, $transliterated);

core/modules/system/system.install

@@ -1730,3 +1730,19 @@ function system_update_8201() {
 /**
  * @} End of "addtogroup updates-8.2.0".
  */
+
+/**
+ * @addtogroup updates-8.2.3
+ * @{
+ */
+
+/**
+ * Clear caches due to behavior change in MachineName element.
+ */
+function system_update_8202() {
+  // Empty update to cause a cache rebuild.
+}
+
+/**
+ * @} End of "addtogroup updates-8.2.3".
+ */

core/modules/system/tests/src/Unit/Transliteration/MachineNameControllerTest.php

@@ -2,9 +2,12 @@
 
 namespace Drupal\Tests\system\Unit\Transliteration;
 
+use Drupal\Core\Access\CsrfTokenGenerator;
 use Drupal\Tests\UnitTestCase;
 use Drupal\Component\Transliteration\PhpTransliteration;
 use Drupal\system\MachineNameController;
+use Prophecy\Argument;
+use Symfony\Component\HttpFoundation\File\Exception\AccessDeniedException;
 use Symfony\Component\HttpFoundation\Request;
 
 /**
@@ -21,10 +24,22 @@ class MachineNameControllerTest extends UnitTestCase {
    */
   protected $machineNameController;
 
+  /**
+   * The CSRF token generator.
+   *
+   * @var \Drupal\Core\Access\CsrfTokenGenerator
+   */
+  protected $tokenGenerator;
+
   protected function setUp() {
     parent::setUp();
     // Create the machine name controller.
-    $this->machineNameController = new MachineNameController(new PhpTransliteration());
+    $this->tokenGenerator = $this->prophesize(CsrfTokenGenerator::class);
+    $this->tokenGenerator->validate(Argument::cetera())->will(function ($args) {
+      return $args[0] === 'token-' . $args[1];
+    });
+
+    $this->machineNameController = new MachineNameController(new PhpTransliteration(), $this->tokenGenerator->reveal());
   }
 
   /**
@@ -38,7 +53,7 @@ protected function setUp() {
    *     - The expected content of the JSONresponse.
    */
   public function providerTestMachineNameController() {
-    return array(
+    $valid_data = array(
       array(array('text' => 'Bob', 'langcode' => 'en'), '"Bob"'),
       array(array('text' => 'Bob', 'langcode' => 'en', 'lowercase' => TRUE), '"bob"'),
       array(array('text' => 'Bob', 'langcode' => 'en', 'replace' => 'Alice', 'replace_pattern' => 'Bob'), '"Alice"'),
@@ -53,6 +68,15 @@ public function providerTestMachineNameController() {
       array(array('text' => 'Bob', 'langcode' => 'en', 'lowercase' => TRUE, 'replace' => 'fail()', 'replace_pattern' => ".*@e\0"), '"bob"'),
       array(array('text' => 'Bob@e', 'langcode' => 'en', 'lowercase' => TRUE, 'replace' => 'fail()', 'replace_pattern' => ".*@e\0"), '"fail()"'),
     );
+
+    $valid_data = array_map(function ($data) {
+      if (isset($data[0]['replace_pattern'])) {
+        $data[0]['replace_token'] = 'token-' . $data[0]['replace_pattern'];
+      }
+      return $data;
+    }, $valid_data);
+
+    return $valid_data;
   }
 
   /**
@@ -73,4 +97,24 @@ public function testMachineNameController(array $request_params, $expected_conte
     $this->assertEquals($expected_content, $json->getContent());
   }
 
+  /**
+   * Tests the pattern validation.
+   */
+  public function testMachineNameControllerWithInvalidReplacePattern() {
+    $request = Request::create('', 'GET', ['text' => 'Bob', 'langcode' => 'en', 'replace' => 'Alice', 'replace_pattern' => 'Bob', 'replace_token' => 'invalid']);
+
+    $this->setExpectedException(AccessDeniedException::class, "Invalid 'replace_token' query parameter.");
+    $this->machineNameController->transliterate($request);
+  }
+
+  /**
+   * Tests the pattern validation with a missing token.
+   */
+  public function testMachineNameControllerWithMissingToken() {
+    $request = Request::create('', 'GET', ['text' => 'Bob', 'langcode' => 'en', 'replace' => 'Alice', 'replace_pattern' => 'Bob']);
+
+    $this->setExpectedException(AccessDeniedException::class, "Missing 'replace_token' query parameter.");
+    $this->machineNameController->transliterate($request);
+  }
+
 }

core/modules/taxonomy/src/Plugin/views/filter/TaxonomyIndexTid.php

@@ -192,7 +192,7 @@ protected function valueForm(&$form, FormStateInterface $form_state) {
           //   https://www.drupal.org/node/1821274.
           ->sort('weight')
           ->sort('name')
-          ->addTag('term_access');
+          ->addTag('taxonomy_term_access');
         if ($this->options['limit']) {
           $query->condition('vid', $vocabulary->id());
         }

core/modules/taxonomy/src/Plugin/views/relationship/NodeTermData.php

@@ -132,7 +132,7 @@ public function query() {
       $query = db_select('taxonomy_term_field_data', 'td');
       $query->addJoin($def['type'], 'taxonomy_index', 'tn', 'tn.tid = td.tid');
       $query->condition('td.vid', array_filter($this->options['vids']), 'IN');
-      $query->addTag('term_access');
+      $query->addTag('taxonomy_term_access');
       $query->fields('td');
       $query->fields('tn', array('nid'));
       $def['table formula'] = $query;

core/modules/taxonomy/src/TermStorage.php

@@ -126,7 +126,7 @@ public function loadParents($tid) {
       $query->addField('t', 'tid');
       $query->condition('h.tid', $tid);
       $query->condition('t.default_langcode', 1);
-      $query->addTag('term_access');
+      $query->addTag('taxonomy_term_access');
       $query->orderBy('t.weight');
       $query->orderBy('t.name');
       if ($ids = $query->execute()->fetchCol()) {
@@ -178,7 +178,7 @@ public function loadChildren($tid, $vid = NULL) {
         $query->condition('t.vid', $vid);
       }
       $query->condition('t.default_langcode', 1);
-      $query->addTag('term_access');
+      $query->addTag('taxonomy_term_access');
       $query->orderBy('t.weight');
       $query->orderBy('t.name');
       if ($ids = $query->execute()->fetchCol()) {
@@ -204,7 +204,7 @@ public function loadTree($vid, $parent = 0, $max_depth = NULL, $load_entities =
         $query = $this->database->select('taxonomy_term_field_data', 't');
         $query->join('taxonomy_term_hierarchy', 'h', 'h.tid = t.tid');
         $result = $query
-          ->addTag('term_access')
+          ->addTag('taxonomy_term_access')
           ->fields('t')
           ->fields('h', array('parent'))
           ->condition('t.vid', $vid)
@@ -320,7 +320,7 @@ public function getNodeTerms(array $nids, array $vocabs = array(), $langcode = N
     $query->orderby('td.weight');
     $query->orderby('td.name');
     $query->condition('tn.nid', $nids, 'IN');
-    $query->addTag('term_access');
+    $query->addTag('taxonomy_term_access');
     if (!empty($vocabs)) {
       $query->condition('td.vid', $vocabs, 'IN');
     }

core/modules/taxonomy/src/TermViewsData.php

@@ -16,7 +16,7 @@ public function getViewsData() {
     $data = parent::getViewsData();
 
     $data['taxonomy_term_field_data']['table']['base']['help'] = $this->t('Taxonomy terms are attached to nodes.');
-    $data['taxonomy_term_field_data']['table']['base']['access query tag'] = 'term_access';
+    $data['taxonomy_term_field_data']['table']['base']['access query tag'] = 'taxonomy_term_access';
     $data['taxonomy_term_field_data']['table']['wizard_id'] = 'taxonomy_term';
 
     $data['taxonomy_term_field_data']['table']['join'] = array(

core/modules/taxonomy/src/Tests/TaxonomyQueryAlterTest.php

+<?php
+
+namespace Drupal\taxonomy\Tests;
+
+use Drupal\simpletest\WebTestBase;
+
+/**
+ * Tests that appropriate query tags are added.
+ *
+ * @group taxonomy
+ */
+class TaxonomyQueryAlterTest extends WebTestBase {
+
+  use TaxonomyTestTrait;
+
+  /**
+   * Modules to enable.
+   *
+   * @var array
+   */
+  public static $modules = ['taxonomy', 'taxonomy_test'];
+
+  /**
+   * Tests that appropriate tags are added when querying the database.
+   */
+  public function testTaxonomyQueryAlter() {
+    // Create a new vocabulary and add a few terms to it.
+    $vocabulary = $this->createVocabulary();
+    $terms = array();
+    for ($i = 0; $i < 5; $i++) {
+      $terms[$i] = $this->createTerm($vocabulary);
+    }
+
+    // Set up hierarchy. Term 2 is a child of 1.
+    $terms[2]->parent = $terms[1]->id();
+    $terms[2]->save();
+
+    $term_storage = \Drupal::entityManager()->getStorage('taxonomy_term');
+
+    $this->setupQueryTagTestHooks();
+    $loaded_term = $term_storage->load($terms[0]->id());
+    $this->assertEqual($loaded_term->id(), $terms[0]->id(), 'First term was loaded');
+    $this->assertQueryTagTestResult(1, 0, 'TermStorage::load()');
+
+    $this->setupQueryTagTestHooks();
+    $loaded_terms = $term_storage->loadTree($vocabulary->id());
+    $this->assertEqual(count($loaded_terms), count($terms), 'All terms were loaded');
+    $this->assertQueryTagTestResult(1, 1, 'TermStorage::loadTree()');
+
+    $this->setupQueryTagTestHooks();
+    $loaded_terms = $term_storage->loadParents($terms[2]->id());
+    $this->assertEqual(count($loaded_terms), 1, 'All parent terms were loaded');
+    $this->assertQueryTagTestResult(2, 1, 'TermStorage::loadParents()');
+
+    $this->setupQueryTagTestHooks();
+    $loaded_terms = $term_storage->loadChildren($terms[1]->id());
+    $this->assertEqual(count($loaded_terms), 1, 'All child terms were loaded');
+    $this->assertQueryTagTestResult(2, 1, 'TermStorage::loadChildren()');
+
+    $this->setupQueryTagTestHooks();
+    $query = db_select('taxonomy_term_data', 't');
+    $query->addField('t', 'tid');
+    $query->addTag('taxonomy_term_access');
+    $tids = $query->execute()->fetchCol();
+    $this->assertEqual(count($tids), count($terms), 'All term IDs were retrieved');
+    $this->assertQueryTagTestResult(1, 1, 'custom db_select() with taxonomy_term_access tag (preferred)');
+
+    $this->setupQueryTagTestHooks();
+    $query = db_select('taxonomy_term_data', 't');
+    $query->addField('t', 'tid');
+    $query->addTag('term_access');
+    $tids = $query->execute()->fetchCol();
+    $this->assertEqual(count($tids), count($terms), 'All term IDs were retrieved');
+    $this->assertQueryTagTestResult(1, 1, 'custom db_select() with term_access tag (deprecated)');
+
+    $this->setupQueryTagTestHooks();
+    $query = \Drupal::entityQuery('taxonomy_term');
+    $query->addTag('taxonomy_term_access');
+    $result = $query->execute();
+    $this->assertEqual(count($result), count($terms), 'All term IDs were retrieved');
+    $this->assertQueryTagTestResult(1, 1, 'custom EntityFieldQuery with taxonomy_term_access tag (preferred)');
+
+    $this->setupQueryTagTestHooks();
+    $query = \Drupal::entityQuery('taxonomy_term');
+    $query->addTag('term_access');
+    $result = $query->execute();
+    $this->assertEqual(count($result), count($terms), 'All term IDs were retrieved');
+    $this->assertQueryTagTestResult(1, 1, 'custom EntityFieldQuery with term_access tag (deprecated)');
+  }
+
+  /**
+   * Sets up the hooks in the test module.
+   */
+  protected function setupQueryTagTestHooks() {
+    taxonomy_terms_static_reset();
+    \Drupal::state()->set('taxonomy_test_query_alter', 0);
+    \Drupal::state()->set('taxonomy_test_query_term_access_alter', 0);
+    \Drupal::state()->set('taxonomy_test_query_taxonomy_term_access_alter', 0);
+  }
+
+  /**
+   * Verifies invocation of the hooks in the test module.
+   *
+   * @param int $expected_generic_invocations
+   *   The number of times the generic query_alter hook is expected to have
+   *   been invoked.
+   * @param int $expected_specific_invocations
+   *   The number of times the tag-specific query_alter hooks are expected to
+   *   have been invoked.
+   * @param string $method
+   *   A string describing the invoked function which generated the query.
+   */
+  protected function assertQueryTagTestResult($expected_generic_invocations, $expected_specific_invocations, $method) {
+    $this->assertIdentical($expected_generic_invocations, \Drupal::state()->get('taxonomy_test_query_alter'), 'hook_query_alter() invoked when executing ' . $method);
+    $this->assertIdentical($expected_specific_invocations, \Drupal::state()->get('taxonomy_test_query_term_access_alter'), 'Deprecated hook_query_term_access_alter() invoked when executing ' . $method);
+    $this->assertIdentical($expected_specific_invocations, \Drupal::state()->get('taxonomy_test_query_taxonomy_term_access_alter'), 'Preferred hook_query_taxonomy_term_access_alter() invoked when executing ' . $method);
+  }
+
+}

core/modules/taxonomy/taxonomy.module

@@ -7,8 +7,8 @@
 
 use Drupal\Component\Utility\Tags;
 use Drupal\Component\Utility\Unicode;
-use Drupal\Core\Entity\Sql\SqlContentEntityStorage;
 use Drupal\Core\Entity\EntityInterface;
+use Drupal\Core\Entity\Sql\SqlContentEntityStorage;
 use Drupal\Core\Render\Element;
 use Drupal\Core\Routing\RouteMatchInterface;
 use Drupal\Core\Url;

core/modules/taxonomy/tests/modules/taxonomy_test/taxonomy_test.info.yml

+name: 'Taxonomy test'
+type: module
+description: 'Provides test hook implementations for taxonomy tests'
+package: Testing
+version: VERSION
+core: 8.x
+dependencies:
+  - taxonomy

core/modules/taxonomy/tests/modules/taxonomy_test/taxonomy_test.module

+<?php
+
+/**
+ * @file
+ * Provides test hook implementations for taxonomy tests.
+ */
+
+use Drupal\Core\Database\Query\AlterableInterface;
+
+/**
+ * Implements hook_query_alter().
+ */
+function taxonomy_test_query_alter(AlterableInterface $query) {
+  $value = \Drupal::state()->get(__FUNCTION__);
+  if (isset($value)) {
+    \Drupal::state()->set(__FUNCTION__, ++$value);
+  }
+}
+
+/**
+ * Implements hook_query_TAG_alter().
+ */
+function taxonomy_test_query_term_access_alter(AlterableInterface $query) {
+  $value = \Drupal::state()->get(__FUNCTION__);
+  if (isset($value)) {
+    \Drupal::state()->set(__FUNCTION__, ++$value);
+  }
+}
+
+/**
+ * Implements hook_query_TAG_alter().
+ */
+function taxonomy_test_query_taxonomy_term_access_alter(AlterableInterface $query) {
+  $value = \Drupal::state()->get(__FUNCTION__);
+  if (isset($value)) {
+    \Drupal::state()->set(__FUNCTION__, ++$value);
+  }
+}

core/modules/user/src/Form/UserPasswordForm.php

@@ -99,6 +99,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
     }
     $form['actions'] = array('#type' => 'actions');
     $form['actions']['submit'] = array('#type' => 'submit', '#value' => $this->t('Submit'));
+    $form['#cache']['contexts'][] = 'url.query_args';
 
     return $form;
   }

core/modules/user/src/Tests/UserPasswordResetTest.php

@@ -292,6 +292,9 @@ public function testUserResetPasswordTextboxFilled() {
     unset($edit['pass']);
     $this->drupalGet('user/password', array('query' => array('name' => $edit['name'])));
     $this->assertFieldByName('name', $edit['name'], 'User name found.');
+    // Ensure the name field value is not cached.
+    $this->drupalGet('user/password');
+    $this->assertNoFieldByName('name', $edit['name'], 'User name not found.');
   }
 
   /**

core/tests/Drupal/Tests/Core/Render/Element/MachineNameTest.php

@@ -2,6 +2,7 @@
 
 namespace Drupal\Tests\Core\Render\Element;
 
+use Drupal\Core\Access\CsrfTokenGenerator;
 use Drupal\Core\Form\FormState;
 use Drupal\Core\Form\FormStateInterface;
 use Drupal\Core\Language\LanguageInterface;
@@ -78,8 +79,12 @@ public function testProcessMachineName() {
     $language_manager = $this->prophesize(LanguageManagerInterface::class);
     $language_manager->getCurrentLanguage()->willReturn($language);
 
+    $csrf_token = $this->prophesize(CsrfTokenGenerator::class);
+    $csrf_token->get('[^a-z0-9_]+')->willReturn('tis-a-fine-token');
+
     $container = $this->prophesize(ContainerInterface::class);
     $container->get('language_manager')->willReturn($language_manager->reveal());
+    $container->get('csrf_token')->willReturn($csrf_token->reveal());
     \Drupal::setContainer($container->reveal());
 
     $element = MachineName::processMachineName($element, $form_state, $complete_form);
@@ -93,7 +98,8 @@ public function testProcessMachineName() {
       'label',
       'field_prefix',
       'field_suffix',
-      'suffix'
+      'suffix',
+      'replace_token',
     ];
     $this->assertEmpty(array_diff_key($settings, array_flip($allowed_options)));
     foreach ($allowed_options as $key) {