Skip to content

Commit

Permalink
Merge 138b336 into df6ebe1
Browse files Browse the repository at this point in the history
  • Loading branch information
antonag32 committed Apr 7, 2022
2 parents df6ebe1 + 138b336 commit 2eecec7
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 2 deletions.
12 changes: 12 additions & 0 deletions pylint_odoo/checkers/no_modules.py
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,11 @@ def _sqli_allowable(self, node):
# sql.SQL or sql.Identifier is OK
if self._is_psycopg2_sql(node):
return True
if isinstance(node, astroid.FormattedValue):
if hasattr(node, 'value'):
return self._sqli_allowable(node.value)
if hasattr(node, 'values'):
return all(self._sqli_allowable(v) for v in node.values)
if isinstance(node, astroid.Call):
node = node.func
# self._thing is OK (mostly self._table), self._thing() also because
Expand Down Expand Up @@ -521,6 +526,13 @@ def _check_node_for_sqli_risk(self, node):
):
return True

# Check fstrings (PEP 498). Only Python >= 3.6
if isinstance(node, astroid.JoinedStr):
if hasattr(node, 'value'):
return self._sqli_allowable(node.value)
elif hasattr(node, 'values'):
return not all(self._sqli_allowable(v) for v in node.values)

return False

def _check_sql_injection_risky(self, node):
Expand Down
39 changes: 37 additions & 2 deletions pylint_odoo/test/main.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@

import os
import stat
import sys
from tempfile import gettempdir
from tempfile import gettempdir, NamedTemporaryFile
import six

import unittest
Expand Down Expand Up @@ -432,6 +431,42 @@ def test_140_check_migrations_is_not_odoo_module(self):
expected_errors = {}
self.assertDictEqual(real_errors, expected_errors)

@unittest.skipUnless(
sys.version_info >= (3, 6),
"Fstrings (PEP498) are only supported since 3.6"
)
def test_145_check_fstring_sqli(self):
"""Verify the linter is capable of finding SQL Injection vulnerabilities
when using fstrings.
Related to https://github.com/OCA/pylint-odoo/issues/363"""
extra_params = [
'--disable=all',
'--enable=sql-injection'
]
queries = '''
def fstring_sqli(self):
self.env.cr.execute(f"SELECT * FROM TABLE WHERE SQLI = {self.table}")
self.env.cr.execute(
f"SELECT * FROM TABLE WHERE SQLI = {'hello' + self.name}"
)
self.env.cr.execute(f"SELECT * FROM {self.name} WHERE SQLI = {'hello'}")
death_wish = f"SELECT * FROM TABLE WHERE SQLI = {self.name}"
self.env.cr.execute(death_wish)
def fstring_no_sqli(self):
self.env.cr.execute(f"SELECT * FROM TABLE WHERE SQLI = {'hello'}")
self.env.cr.execute(
f"CREATE VIEW {self._table} AS (SELECT * FROM res_partner)"
)
self.env.cr.execute(f"SELECT NAME FROM res_partner LIMIT 10")
'''
with NamedTemporaryFile(mode='w') as f:
f.write(queries)
f.flush()
pylint_res = self.run_pylint([f.name], extra_params)

real_errors = pylint_res.linter.stats['by_msg']
self.assertDictEqual(real_errors, {'sql-injection': 4})


if __name__ == '__main__':
unittest.main()

0 comments on commit 2eecec7

Please sign in to comment.