Originally reported to the Drupal security team by
@kristiaanvandeneynde →
on 28 August 2020 (#173376). Assuming it affects the latest version, this issue's version is set to D10.1
---
(Original post)
This module has an information disclosure vulnerability.
You can see this vulnerability by:
1. Having some form of generic entity query access that also applies to nodes
2. Having books that are not supposed to be public (also applies to tracker/forum/etc.)
3. Visiting /book and still seeing the book titles
This is caused because /book calls BookController::bookRender(), which in turn calls BookManager::getAllBooks(). In that method, a call to BookManager::loadBooks() is made, where node IDs are retrieved without checking entity query access.
BookOutlineStorage::loadMultiple() runs a direct query against the book table and adds the following:
- Tag: node_access
- Metadata: base_table = book
The problem is that this will ONLY trigger node_query_node_access_alter(). So any module that defines node access using grants, will be secure. But any module defining access using hook_query_entity_query_alter() will not be taken into account.
This is because hook_query_entity_query_alter() triggers for entity queries (including node), but has the following expectations:
- Tag: node_access
- Metadata: entity_type = node
These expectations come from the entity query class:
$this->sqlQuery->addMetaData('entity_type', $this->entityTypeId);
// ...
if ($this->accessCheck) {
$this->sqlQuery->addTag($this->entityTypeId . '_access');
}
$this->sqlQuery->addTag('entity_query');
$this->sqlQuery->addTag('entity_query_' . $this->entityTypeId);
So leveraging the "base_table" metadata and expecting all queries to be secure is an unexpected quirk for modules who care about ALL entities rather than just nodes. If you look at Entity API (and as an extension, Group), you'll find that we only check access as follows:
/**
* Implements hook_query_TAG_alter().
*/
function entity_query_entity_query_alter(SelectInterface $query) {
$entity_type_id = $query->getMetaData('entity_type');
if ($query->hasTag($entity_type_id . '_access')) {
// ... Access checks go here ...
Turns out, the above doesn't trigger because node.module is the only module that allows you to attach said query access checks to tables other than the actual entity's table. To add insult to injury it also hard-codes the expectation of a "nid" column being present on said table.
Needless to say, this leads to a security issue in Entity API, Group and any other module implementing generic entity query access, so I'm not sure under which module to file this report, leading me to mark it as a "Drupal core" issue.
Combine this with https://security.drupal.org/node/173249 and hopefully you'll understand why I'm really starting to hate developing a generic entity access module such as Group because node just keeps on raining on my parade at every chance it gets.
Can we PLEASE start removing all of the node-specific quirks from core and have the entire entity ecosystem follow the same expectations?
Problem/Motivation
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Contributors
- kristiaanvandeneynde
- David_Rothstein
- catch
Coordination:
- larowlan
- greggles