Problem/Motivation
A security review identified multiple locations throughout the CRM module where accessCheck(FALSE) is used in entity queries. This disables Drupal's entity-level access control system, which could expose data to users who shouldn't have access to it.
Security Impact: While most affected routes are protected by administrative permissions (e.g., "administer crm"), this creates a defense-in-depth issue. Entity queries should respect entity-level access controls in addition to route-level permissions to prevent information disclosure if:
- Additional access restrictions are added via hook_entity_access()
- Custom permissions are configured on entities
- The code is reused in contexts with different route permissions
Current behavior: Users with route-level access can see counts and data for ALL entities, even those they shouldn't have entity-level access to view.
Affected locations:
src/Controller/RelationshipController.php(lines 110, 134)src/Access/ContactAccessControlHandler.php(line 145)src/ListBuilder/ContactListBuilder.php(line 48)src/ListBuilder/ContactMethodListBuilder.php(lines 79, 133)src/ListBuilder/RelationshipListBuilder.php(line 48)src/Service/RelationshipStatisticsService.php(lines 152, 216)src/Plugin/Validation/Constraint/RelationshipLimitConstraintValidator.php(line 128)src/Form/RelationshipTypeForm.php(line 367)src/Hook/ContactHooks.php(line 185)
Steps to reproduce
- Install the CRM module
- Create a custom access handler or use hook_entity_access() to restrict certain contacts
- Create a user with "administer crm" permission but restricted entity-level access
- Log in as that user and navigate to
/admin/crm/contacts - Observe that the entity counts include contacts the user shouldn't be able to view
- Navigate to relationship statistics pages
- Observe statistics include relationships for contacts the user cannot access
Expected result: Entity queries should only return entities the current user has permission to view, respecting both route-level AND entity-level access controls.
Actual result: Entity queries with accessCheck(FALSE) return all entities regardless of entity-level access restrictions, potentially exposing sensitive data.
Proposed resolution
Review all instances of accessCheck(FALSE) and categorize them into three groups, handling each appropriately:
Implementation approach:
- Controllers and List Builders: Change to
accessCheck(TRUE)to respect entity-level permissions. Example:
// Before:
$query = $this->entityTypeManager
->getStorage('crm_contact')
->getQuery()
->accessCheck(FALSE);
// After:
$query = $this->entityTypeManager
->getStorage('crm_contact')
->getQuery()
->accessCheck(TRUE);
- Internal Services: Keep
accessCheck(FALSE)for system-level statistics where access has already been verified at the route level. Add inline comments:
// Deliberately bypassing access checks because this is a system-level
// calculation. Route-level access is enforced by the 'administer crm'
// permission on the calling controller.
$query = $this->entityTypeManager
->getStorage('crm_relationship')
->getQuery()
->accessCheck(FALSE);
- Validation Constraints: Keep
accessCheck(FALSE)as validators run in a system context. Add comments explaining this. - Access Handlers: Review case-by-case, as these are part of the access determination logic itself.
Backward compatibility: This change will reduce the entity counts shown to users with limited permissions. This is the correct behavior and shouldn't break any functionality, but should be noted in the release notes.
Remaining tasks
- Review and update
src/Controller/RelationshipController.php - Review and update
src/ListBuilder/ContactListBuilder.php - Review and update
src/ListBuilder/ContactMethodListBuilder.php - Review and update
src/ListBuilder/RelationshipListBuilder.php - Review
src/Form/RelationshipTypeForm.php - Review
src/Hook/ContactHooks.php - Review
src/Access/ContactAccessControlHandler.php - Add explanatory comments to
src/Service/RelationshipStatisticsService.php - Add explanatory comments to
src/Plugin/Validation/Constraint/RelationshipLimitConstraintValidator.php - Write automated tests verifying access control enforcement with restricted users
- Write tests for legitimate
accessCheck(FALSE)usage
User interface changes
No direct UI changes. List builders may show different entity counts for users with limited permissions after the fix.
API changes
None. This is an internal security fix that doesn't change public APIs.
Data model changes
None.
Issue fork crm-3571870
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:
Comments
Comment #4
bluegeek9 commentedComment #5
bluegeek9 commented