Odoo E8103: SQL injection risk. Use parameters if you can

Question

I'm using odoo pylint and encountered this message 'Odoo E8103: SQL injection risk. Use parameters if you can'. It was referring to these following example SQL query line pattern:

 self.env.cr.execute("""SELECT sq_qty , prod_id ...
        FROM st
        LEFT JOIN prod
        ON prod.id = st.prod_id
        WHERE %s
        GROUP BY prod_id, loc_id, ..."""% domain, args)

and according to OCA No SQL Injection, the best way to be safe is to never, NEVER use Python string concatenation (+) or string parameters interpolation (%) to pass variables to a SQL query string.

I tried replacing '%' with ',' and enclose domain and args with (): GROUP BY prod_id, loc_id, ...""", (domain, args)), it passed the checks but not sure if this was the correct way.

Is placing skip check be better or modify it with what the pylint suggested and if it's better to modify, what is the correct way to handle it?

Edit: I separated the sql query statement as a variable

query = """SELECT sq_qty , prod_id ...
        FROM st
        LEFT JOIN prod
        ON prod.id = st.prod_id
        WHERE """
query += domain + """ GROUP BY prod_id, loc_id, ..."""
self.env.cr.execute(query, args)

Using Classic approach since I'm using dynamic WHERE clause with domain and following OCA description, achieving both passing the lint and running without errors

score 1 · Accepted Answer · answered Nov 17 '20 at 10:13

You've changed it correctly. Never ever ignore that lint message, if you're not fully sure about the reasons behind it.

The OCA description already have the correct answers, why to avoid such code.

Care must be taken not to introduce SQL injections vulnerabilities when using manual SQL queries. The vulnerability is present when user input is either incorrectly filtered or badly quoted, allowing an attacker to introduce undesirable clauses to a SQL query (such as circumventing filters or executing UPDATE or DELETE commands).

The best way to be safe is to never, NEVER use Python string concatenation (+) or string parameters interpolation (%) to pass variables to a SQL query string.

The second reason, which is almost as important, is that it is the job of the database abstraction layer (psycopg2) to decide how to format query parameters, not your job! For example psycopg2 knows that when you pass a list of values it needs to format them as a comma-separated list, enclosed in parentheses!

Even the good old comic "Bobby tables" from xkcd is mentioned ;-)

Odoo E8103: SQL injection risk. Use parameters if you can

1 Answers1