-
Notifications
You must be signed in to change notification settings - Fork 0
Open
Description
Summary
The listEntities() method accepts a $conditions array that is passed directly to entity query conditions. While this is secure against SQL injection (Drupal's query API handles escaping), it could allow unintended field filtering if exposed to untrusted input.
Current Implementation
File: src/Service/ContentIntelCollector.php
Lines: 147-173
public function listEntities(
string $entity_type_id,
?string $bundle = NULL,
int $limit = 50,
int $offset = 0,
array $conditions = [], // <-- Accepts arbitrary conditions
): array {
// ...
foreach ($conditions as $field => $value) {
$query->condition($field, $value); // Applied directly
}
}Current Usage
The $conditions parameter is not exposed in Drush commands - it's only available when calling the service programmatically. This is appropriate.
Recommendations
1. Add PHPDoc Security Note
/**
* @param array $conditions
* Additional query conditions. IMPORTANT: Only pass conditions from
* trusted sources. While SQL injection is prevented by Drupal's query
* API, arbitrary field conditions could expose unintended data filtering
* capabilities. Do not pass user input directly to this parameter.
*/2. Consider Condition Validation (Optional)
For extra safety, validate that conditions only use allowed fields:
$allowed_fields = ['status', 'uid', 'created', 'changed'];
foreach ($conditions as $field => $value) {
if (!in_array($field, $allowed_fields, TRUE)) {
throw new \InvalidArgumentException("Condition field '$field' not allowed.");
}
$query->condition($field, $value);
}Risk Assessment
Current Risk: LOW
- Parameter is not exposed to CLI users
- Drupal's query API prevents SQL injection
- Only trusted code (other modules) can pass conditions
This issue is primarily about documentation to ensure future developers understand the security considerations.
Metadata
Metadata
Assignees
Labels
No labels