mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Declare func() as safe method in phan 

We added a special `func()` method to the query builder, which is a plain text function by definition. It uses the string and does no escaping on purpose. It has the potential for an injection but requiring to add the "supress warning" to all surrounding code makes it harder to spot actual problems, that this plugin want to find. So it's better to only need to check the func() and not all the surrounding code as well.

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
This commit is contained in:
Morris Jobke 2018-04-16 14:56:54 +02:00
parent 056660bf7c
commit 1f06bc246c
No known key found for this signature in database
GPG Key ID: FE03C3A163FEDE68
2 changed files with 17 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
build/.phan/plugin-checker.php
View File

@ -20,17 +20,17 @@
*/
$expected = <<<EOT
build/.phan/tests/SqlInjectionCheckerTest.php:23 SqlInjectionChecker Potential SQL injection detected
build/.phan/tests/SqlInjectionCheckerTest.php:35 SqlInjectionChecker Potential SQL injection detected
build/.phan/tests/SqlInjectionCheckerTest.php:37 SqlInjectionChecker Potential SQL injection detected
build/.phan/tests/SqlInjectionCheckerTest.php:39 SqlInjectionChecker Potential SQL injection detected
build/.phan/tests/SqlInjectionCheckerTest.php:41 SqlInjectionChecker Potential SQL injection detected
build/.phan/tests/SqlInjectionCheckerTest.php:43 SqlInjectionChecker Potential SQL injection detected
build/.phan/tests/SqlInjectionCheckerTest.php:54 SqlInjectionChecker Potential SQL injection detected
build/.phan/tests/SqlInjectionCheckerTest.php:61 SqlInjectionChecker Potential SQL injection detected
build/.phan/tests/SqlInjectionCheckerTest.php:62 SqlInjectionChecker Potential SQL injection detected
build/.phan/tests/SqlInjectionCheckerTest.php:69 SqlInjectionChecker Potential SQL injection detected
build/.phan/tests/SqlInjectionCheckerTest.php:70 SqlInjectionChecker Potential SQL injection detected
build/.phan/tests/SqlInjectionCheckerTest.php:23 SqlInjectionChecker Potential SQL injection detected - neither a parameter nor a string
build/.phan/tests/SqlInjectionCheckerTest.php:35 SqlInjectionChecker Potential SQL injection detected - neither a parameter nor a string
build/.phan/tests/SqlInjectionCheckerTest.php:37 SqlInjectionChecker Potential SQL injection detected - neither a parameter nor a string
build/.phan/tests/SqlInjectionCheckerTest.php:39 SqlInjectionChecker Potential SQL injection detected - neither a parameter nor a string
build/.phan/tests/SqlInjectionCheckerTest.php:41 SqlInjectionChecker Potential SQL injection detected - neither a parameter nor a string
build/.phan/tests/SqlInjectionCheckerTest.php:43 SqlInjectionChecker Potential SQL injection detected - neither a parameter nor a string
build/.phan/tests/SqlInjectionCheckerTest.php:54 SqlInjectionChecker Potential SQL injection detected - neither a parameter nor a string
build/.phan/tests/SqlInjectionCheckerTest.php:61 SqlInjectionChecker Potential SQL injection detected - method: no child method
build/.phan/tests/SqlInjectionCheckerTest.php:62 SqlInjectionChecker Potential SQL injection detected - method: no child method
build/.phan/tests/SqlInjectionCheckerTest.php:69 SqlInjectionChecker Potential SQL injection detected - method: no child method
build/.phan/tests/SqlInjectionCheckerTest.php:70 SqlInjectionChecker Potential SQL injection detected - method: no child method
EOT;

10
build/.phan/plugins/SqlInjectionCheckerPlugin.php
View File

@ -33,10 +33,10 @@ class SqlInjectionCheckerPlugin extends PluginV2 implements AnalyzeNodeCapabili
class SqlInjectionCheckerVisitor extends PluginAwareAnalysisVisitor {
private function throwError() {
private function throwError(string $hint) {
$this->emit(
'SqlInjectionChecker',
'Potential SQL injection detected',
'Potential SQL injection detected - ' . $hint,
[],
\Phan\Issue::SEVERITY_CRITICAL
);
@ -64,6 +64,8 @@ class SqlInjectionCheckerVisitor extends PluginAwareAnalysisVisitor {
'createNamedParameter',
'createPositionalParameter',
'createParameter',
'createFunction',
'func',
];
$functionsToSearch = [
@ -84,7 +86,7 @@ class SqlInjectionCheckerVisitor extends PluginAwareAnalysisVisitor {
// For set actions
if(isset($node->children['method']) && in_array($node->children['method'], $functionsToSearch, true) && !is_string($subChild)) {
if(!isset($subChild->children['method']) || !in_array($subChild->children['method'], $safeFunctions, true)) {
$this->throwError();
$this->throwError('method: ' . ($subChild->children['method'] ?? 'no child method'));
}
}
@ -115,7 +117,7 @@ class SqlInjectionCheckerVisitor extends PluginAwareAnalysisVisitor {
// If it is an IParameter or a pure string no error is thrown
if((string)$expandedNode !== '\OCP\DB\QueryBuilder\IParameter' && !is_string($secondParameterNode)) {
$this->throwError();
$this->throwError('neither a parameter nor a string');
}
}
}