Problem/Motivation
This was originally reported as a private security issue but cleared by the security team for a public issue
A user with the View all revisions or [bundle]: View revisions permissions can access the Revisions page for a Node and see all revisions listed even when they DO NOT have access to view the Node.
Should they? The description associated with these permissions states To view a revision, you also need permission to view the content item. I am assuming this should also apply to the Revisions page and list?
The date and author of all revisions, along with the node title, are listed on the Node Revisions page. Although this info is relatively mundane it could be compromising depending on the context and what info is included in the title.
Steps to reproduce
- Perform a fresh install of Drupal 10.5.2
- Check the Content editor role has the View all revisions permission. They should by default.
- Create a new user and assign then the Content editor role. The new user's ID should be 2.
- Create an Article Node. The new node's ID should be 1.
- Edit the node to create another revision.
- Create a custom module that contains a Hook class with the following node_access hook which will deny User ID 2 (the user just created) from viewing Node ID 1.
- Enable the custom module.
- Login as the new Content editor user.
- Visit the node view page (e.g. /node/1) -> Access should be correctly denied.
- Visit a specific node revision (e.g. /node/1/revisions/1/view) -> Access should be correctly denied.
- Visit the node's Revisions page (e.g. /node/1/revisions) -> Access is allowed and the date and author of all revisions, along with the node title, are listed.
- Edit permission and remove the View all revisions permission and given the Article: View revisions to the Content editor role.
- Clear site cache.
- Visit the node's Revisions page (e.g. /node/1/revisions) -> Access is allowed and the date and author of all revisions, along with the node title, are listed.
/**
* Implements hook_ENTITY_TYPE_access().
*/
function node_access_test_node_access(NodeInterface $node, $operation, AccountInterface $account): AccessResultInterface {
// Hide Node ID 1 from User ID 2.
if ($operation === 'view' && (int) $node->id() === 1 && (int) $account->id() === 2) {
return AccessResult::forbidden()->setCacheMaxAge(0);
}
// No opinion.
return AccessResult::neutral()->setCacheMaxAge(0);
}
Comments
Comment #2
acbramley commentedIt looks like this is in line with BlockContentAccessControlHandler, but MediaAccessControlHandler does check
viewaccess to the entity AND the default revision. I'm not sure which is more correct, tbh I'd be inclined to update the permission description to make it clearer that the operations are separate.