Merge 2cb506c into 0829a0e

pylint_odoo/checkers/no_modules.py

@@ -447,7 +447,10 @@ def _sqli_allowable(self, node):
         # it's a common pattern of reports (self._select, self._group_by, ...)
         return (isinstance(node, astroid.Attribute)
                 and isinstance(node.expr, astroid.Name)
-                and node.attrname.startswith('_'))
+                and node.attrname.startswith('_')
+                # cr.execute('SELECT * FROM %s' % 'table') is OK
+                # since that is a constant and constant can not be injected
+                or isinstance(node, astroid.Const))
 
     def _is_psycopg2_sql(self, node):
         if isinstance(node, astroid.Name):
@@ -459,7 +462,8 @@ def _is_psycopg2_sql(self, node):
             return False
         imported_name = node.func.as_string().split('.')[0]
         imported_node = node.root().locals.get(imported_name)
-        # TODO: Consider "from psycopg2 import *"?
+        # "from psycopg2 import *" not considered since that it is hard
+        # and there is another check detecting these kind of imports
         if not imported_node:
             return None
         imported_node = imported_node[0]
@@ -474,6 +478,21 @@ def _is_psycopg2_sql(self, node):
 
     def _check_node_for_sqli_risk(self, node):
         if isinstance(node, astroid.BinOp) and node.op in ('%', '+'):
+            if (isinstance(node.left, astroid.BinOp) and
+                    self._check_node_for_sqli_risk(node.left)):
+                # Consider cr.execute('SELECT ' + operator + ' FROM table' + 'WHERE')"
+                # node.repr_tree()
+                # BinOp(
+                #    op='+',
+                #    left=BinOp(
+                #       op='+',
+                #       left=BinOp(
+                #          op='+',
+                #          left=Const(value='SELECT '),
+                #          right=Name(name='operator')),
+                #       right=Const(value=' FROM table')),
+                #    right=Const(value='WHERE'))
+                return True
             if isinstance(node.right, astroid.Tuple):
                 # execute("..." % (self._table, thing))
                 if not all(map(self._sqli_allowable, node.right.elts)):

pylint_odoo/test_repo/broken_module/models/broken_model.py

@@ -527,6 +527,12 @@ def sql_no_injection_private_methods(self):
             )
         )
 
+    def sql_no_injection_constants(self):
+        self.env.cr.execute("SELECT * FROM %s" % 'table_constant')
+        self.env.cr.execute("SELECT * FROM {}".format('table_constant'))
+        self.env.cr.execute(
+            "SELECT * FROM %(table_variable)s" % {'table_variable': 'table_constant'})
+
     def func(self, a):
         length = len(a)
         return length