Skip to content

Commit 66cb7f1

Browse files
Mingsong HuMingsong Hu
authored andcommitted
Add warning of access checking for loadMultiple() and loadByProperties() functions.
1 parent ef5fe3f commit 66cb7f1

File tree

2 files changed

+29
-3
lines changed

2 files changed

+29
-3
lines changed

DrupalSecurity/Sniffs/Database/SqlSniff.php

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@ class SqlSniff implements Sniff {
3232
protected $targetFunctions = [
3333
'query' => true,
3434
'condition' => true,
35-
'accesscheck' => true,
35+
'accessCheck' => true,
36+
'loadMultiple' => true,
37+
'loadByProperties' => true,
3638
];
3739

3840
/**
@@ -59,7 +61,7 @@ public function register() {
5961
*/
6062
public function process(File $phpcsFile, $stackPtr) {
6163
$tokens = $phpcsFile->getTokens();
62-
$functionLc = \strtolower($tokens[$stackPtr]['content']);
64+
$functionLc = $tokens[$stackPtr]['content'];
6365
if (isset($this->targetFunctions[$functionLc]) === false) {
6466
return;
6567
}
@@ -95,12 +97,20 @@ public function process(File $phpcsFile, $stackPtr) {
9597
}
9698
}
9799
break;
98-
case 'accesscheck':
100+
case 'accessCheck':
99101
if (isset($parameters[1]) && \strtolower($parameters[1]['clean']) == 'false') {
100102
$warning = "Query without having access check. @see https://www.drupal.org/node/3201242";
101103
$phpcsFile->addWarning($warning, $stackPtr, 'Access check');
102104
}
103105
break;
106+
case 'loadMultiple':
107+
$warning = "loadMultiple() function detected. This function won't check access during loading. @see https://www.drupal.org/docs/drupal-apis/entity-api/working-with-the-entity-api#s-checking-if-a-user-account-has-access-to-an-entity-object";
108+
$phpcsFile->addWarning($warning, $stackPtr, 'Access check');
109+
break;
110+
case 'loadByProperties':
111+
$warning = "loadByProperties() function detected. This function won't check access during loading. @see https://www.drupal.org/docs/drupal-apis/entity-api/working-with-the-entity-api#s-checking-if-a-user-account-has-access-to-an-entity-object";
112+
$phpcsFile->addWarning($warning, $stackPtr, 'Access check');
113+
break;
104114
}
105115
}// end process()
106116
}//end class

DrupalSecurity/Test/Database/SqlSniff.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,22 @@
2525
->condition('type', 'article')
2626
->execute();
2727

28+
// Mutiple entities loading won't check the access. You have to check the access for those entities after loading.
29+
$entities = \Drupal::entityTypeManager()->getStorage($entityType)->loadMultiple();
30+
31+
32+
$properties = [
33+
$entityStorage->getEntityType()->getKey('bundle') => $bundleID,
34+
];
35+
// Load entities by their property values without any access checks. @see https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Entity%21EntityStorageBase.php/function/EntityStorageBase%3A%3AloadByProperties/10
36+
$entities = $entityStorage->loadByProperties($properties);
37+
38+
// Use the access() function to check the access for each entity.
39+
foreach ($entities as $entity) {
40+
if ($entity->access('view')) {
41+
}
42+
}
43+
2844
// Safe
2945
\Database::getConnection()->query('SELECT foo FROM {table} t WHERE t.name = :name', [':name' => $_GET['user']]);
3046

0 commit comments

Comments
 (0)