This was determined by the Security team to be suitable to post publicly

CommentDefaultFormatter does not check any access control and assumes the default permissions like 'access comments' are the only access logic in play.

You can reproduce this problem by:

  1. Enabling the module
  2. Write a custom access control handler for comment entities. Use hook_entity_type_info_alter to wire up this handler for comment entities.
  3. Create an entity with a comment field on it. Add some comments. On the comment thread, note that the permission 'access comments' is used instead of deferring to the custom comment access control handler.

Commentary from Berdir:

The problem is that we have no concept of a "can access list of entities" operation, that simply doesn't exist and would need to be handled specially, like create access, as there is no entity to operate on/with, so we can not simply introduce a new operation.

Given that there is no "proper" way to do it, not sure if this is a security issue.

There are a few other as well, for example checking for being able to create comments, that's something we could do better. But even that sounds like a normal or maybe a security hardening bug to me.

CommentDefaultFormatter itself is also just a plugin, so if you have a case like this where you want to customize comment access, you could use a different formatter...

Many other listing pages are equally hardcoded (although this is arguably more exposed than most default (config) entity listings. The only thing that comes close to the concept of overview-access is having an admin permission, see \Drupal\Core\Entity\Routing\DefaultHtmlRouteProvider::getCollectionRoute().

And last, \Drupal\comment\CommentStorage::loadThread() implements comment_filter and entity_access (this is arguably a bit weird, since we always use ${entity_type}_access as tag. So you can for example add a condition there to deny access if the host is a certain entity or entity type.

original report:

I customized the view access logic for comments by setting my own access class. This worked for viewing a comment at comment/{comment} but not when viewing a list of comments in a comment field.

CommentDefaultFormatter does not check any access control and assumes the default permissions like 'access comments' are the only access logic in play.

Issue fork drupal-2858157

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Jody Lynn created an issue. See original summary.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

Going to make this private. Will add you over there.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

Let's continue this in https://security.drupal.org/node/161916

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

pwolanin’s picture

pwolanin commented
pwolanin’s picture

pwolanin commented
Title: Comment display does not allow custom comment access control » CommentDefaultFormatter does not respect custom access control
Issue summary: View changes
Issue tags: +Security improvements

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

a_grizzli’s picture

a_grizzli commented
Issue summary: View changes

I struggled with same problem, when trying to restrict access to some type of comments. So implementing a custom access handler for comment entity only protects comment/{comment} path, but not comment listings. In my case it was possible to adapt query in a proper way using hook_query_alter and looking for comment_filter tag:

/**
 * Implements hook_query_alter
 */
function mymodule_query_alter(Drupal\Core\Database\Query\AlterableInterface $query) {

  // Only for especially tagged select queries
  if($query->hasTag('comment_filter')) {

    // If current user has restricted permissions on accessing special comments
    $user = \Drupal::currentUser();
    if(MyCustomAccessHandler::checkCommentsAccessPrecondition(...)) {

      // Then add filter to query, to exclude some comments
      $query->condition(...);
    }
  }
}

In general, it might be problematic to combine custom access check on single comment entity and DB queries for comment list.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented

I imagine #777578: Add an entity query access API and deprecate hook_query_ENTITY_TYPE_access_alter() will eventually help with this.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

prudloff made their first commit to this issue’s fork.

prudloff’s picture

prudloff commented
Related issues: +#777578: Add an entity query access API and deprecate hook_query_ENTITY_TYPE_access_alter()

I'm not sure this can be fixed in CommentDefaultFormatter.

If #777578: Add an entity query access API and deprecate hook_query_ENTITY_TYPE_access_alter() adds a generic system to alter access in entity queries, then custom code could alter access in comment queries and it would hopefully apply everywhere including in CommentStorage::loadThread().

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.