aboutsummaryrefslogtreecommitdiffstats
path: root/build
diff options
context:
space:
mode:
authorMorris Jobke <hey@morrisjobke.de>2018-04-16 14:56:54 +0200
committerMorris Jobke <hey@morrisjobke.de>2018-04-17 10:58:00 +0200
commit1f06bc246c1de15d835ea563b5d5c4f820fa6df8 (patch)
tree5f80efacd76150a9801887696551799cd19ffaeb /build
parent056660bf7ce0e587be7276e640e424280ff66804 (diff)
downloadnextcloud-server-1f06bc246c1de15d835ea563b5d5c4f820fa6df8.tar.gz
nextcloud-server-1f06bc246c1de15d835ea563b5d5c4f820fa6df8.zip
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>
Diffstat (limited to 'build')
-rw-r--r--build/.phan/plugin-checker.php22
-rw-r--r--build/.phan/plugins/SqlInjectionCheckerPlugin.php10
2 files changed, 17 insertions, 15 deletions
diff --git a/build/.phan/plugin-checker.php b/build/.phan/plugin-checker.php
index 92eb3496ed5..f7946fc2a42 100644
--- a/build/.phan/plugin-checker.php
+++ b/build/.phan/plugin-checker.php
@@ -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;
diff --git a/build/.phan/plugins/SqlInjectionCheckerPlugin.php b/build/.phan/plugins/SqlInjectionCheckerPlugin.php
index 8cfd5ac4752..a9a0b817d5c 100644
--- a/build/.phan/plugins/SqlInjectionCheckerPlugin.php
+++ b/build/.phan/plugins/SqlInjectionCheckerPlugin.php
@@ -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');
}
}
}