From 1f06bc246c1de15d835ea563b5d5c4f820fa6df8 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Mon, 16 Apr 2018 14:56:54 +0200 Subject: [PATCH] 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 --- build/.phan/plugin-checker.php | 22 +++++++++---------- .../plugins/SqlInjectionCheckerPlugin.php | 10 +++++---- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/build/.phan/plugin-checker.php b/build/.phan/plugin-checker.php index 92eb3496ed..f7946fc2a4 100644 --- a/build/.phan/plugin-checker.php +++ b/build/.phan/plugin-checker.php @@ -20,17 +20,17 @@ */ $expected = <<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'); } } }