fix: disallow global variable access through sql (#10875)
* fix: disallow global variable access through sql Signed-off-by: Chinmay D. Pai <chinmaydpai@gmail.com> Co-authored-by: Sahil Khan <sahilkhan28297@gmail.com> * chore: add test for sql disallowed variable access Signed-off-by: Chinmay D. Pai <chinmaydpai@gmail.com> Co-authored-by: Sahil Khan <sahilkhan28297@gmail.com>
This commit is contained in:
parent
daa479434a
commit
e30161b222
2 changed files with 12 additions and 5 deletions
|
|
@ -203,7 +203,7 @@ class DatabaseQuery(object):
|
|||
def sanitize_fields(self):
|
||||
'''
|
||||
regex : ^.*[,();].*
|
||||
purpose : The regex will look for malicious patterns like `,`, '(', ')', ';' in each
|
||||
purpose : The regex will look for malicious patterns like `,`, '(', ')', '@', ;' in each
|
||||
field which may leads to sql injection.
|
||||
example :
|
||||
field = "`DocType`.`issingle`, version()"
|
||||
|
|
@ -211,11 +211,11 @@ class DatabaseQuery(object):
|
|||
the system will filter out this field.
|
||||
'''
|
||||
|
||||
sub_query_regex = re.compile("^.*[,();].*")
|
||||
blacklisted_keywords = ['select', 'create', 'insert', 'delete', 'drop', 'update', 'case']
|
||||
sub_query_regex = re.compile("^.*[,();@].*")
|
||||
blacklisted_keywords = ['select', 'create', 'insert', 'delete', 'drop', 'update', 'case', 'show']
|
||||
blacklisted_functions = ['concat', 'concat_ws', 'if', 'ifnull', 'nullif', 'coalesce',
|
||||
'connection_id', 'current_user', 'database', 'last_insert_id', 'session_user',
|
||||
'system_user', 'user', 'version']
|
||||
'system_user', 'user', 'version', 'global']
|
||||
|
||||
def _raise_exception():
|
||||
frappe.throw(_('Use of sub-query or function is restricted'), frappe.DataError)
|
||||
|
|
@ -238,6 +238,10 @@ class DatabaseQuery(object):
|
|||
if any("{0}(".format(keyword) in field.lower() for keyword in blacklisted_functions):
|
||||
_raise_exception()
|
||||
|
||||
if '@' in field.lower():
|
||||
# prevent access to global variables
|
||||
_raise_exception()
|
||||
|
||||
if re.compile(r"[0-9a-zA-Z]+\s*'").match(field):
|
||||
_raise_exception()
|
||||
|
||||
|
|
@ -854,4 +858,4 @@ def get_date_range(operator, value):
|
|||
|
||||
timespan = period_map[operator] + ' ' + timespan_map[value] if operator != 'timespan' else value
|
||||
|
||||
return get_timespan_date_range(timespan)
|
||||
return get_timespan_date_range(timespan)
|
||||
|
|
|
|||
|
|
@ -167,6 +167,9 @@ class TestReportview(unittest.TestCase):
|
|||
self.assertRaises(frappe.DataError, DatabaseQuery("DocType").execute,
|
||||
fields=["name", "1' UNION SELECT * FROM __Auth --"],limit_start=0, limit_page_length=1)
|
||||
|
||||
self.assertRaises(frappe.DataError, DatabaseQuery("DocType").execute,
|
||||
fields=["@@version"], limit_start=0, limit_page_length=1)
|
||||
|
||||
data = DatabaseQuery("DocType").execute(fields=["count(`name`) as count"],
|
||||
limit_start=0, limit_page_length=1)
|
||||
self.assertTrue('count' in data[0])
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue