False positives with sql-injection check

[joao-p-marques]

The sql-injection check is currently detecting some false positives.

For example, if you are building an Odoo report, you will end up with something like:

    def init(self):
        tools.drop_view_if_exists(self.env.cr, self._table)
        self.env.cr.execute(
            """
            CREATE OR REPLACE VIEW %s AS (
                %s %s %s %s
            )
        """
            % (
                self._table,
                self._select(),
                self._from(),
                self._where(),
                self._group_by(),
            )
        )

where a SQL query is constructed both from private attributes and private methods.

This is the standard behavior implemented by Odoo in many reports: https://github.com/odoo/odoo/blob/13.0/addons/project/report/project_report.py#L93

If you have something like the example above, you will get a linter error warning about the danger of SQLi. However, should this be treated as an actual security issue? It is true that methods can introduce a larger security concern than attributes, which are already ignored (https://github.com/OCA/pylint-odoo/pull/122), but in a case like this you will always be forced to completely ignore the linter for that block, which is not a pretty solution.

I don't know if, as this is the standard Odoo behavior and is somewhat controlled, cases like this could also be ignored from the linter. WDYT @moylop260 @nilshamerlinck @Yajo?

[moylop260]

@joao-p-marques

You are right!

We have already fixed all them in the following PR:

• https://github.com/odoo/odoo/pull/36583

We need to update the following file in OCA pylint-odoo:

• https://github.com/odoo/odoo/blob/14.0/odoo/addons/test_lint/tests/_odoo_checker_sql_injection.py

Help wanted

[voronind]

It's not false positive. We need use AsIs function