From da9645c9253936a14db7f8fc7f9c04b596baa602 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 5 Aug 2020 19:59:44 +0530 Subject: [PATCH] feat: Add validate_and_sanitize_search_inputs decorator (#11194) (#11204) * feat: Add validate_and_sanitize_search_inputs decorator Co-authored-by: Nabin Hait Co-authored-by: Prssanna Desai Co-authored-by: Shivam Mishra * refactor: Move validate_and_sanitize_search_inputs to init * refactor: Move validate_and_sanitize_search_inputs to search.py * test: validate_and_sanitize_search_inputs decorator * chore: Add wrapt module * refactor: Use @wrapt to define validate_and_sanitize_search_inputs decorator * test: Add a case to make sure frappe.call works as well Co-authored-by: Nabin Hait Co-authored-by: Prssanna Desai Co-authored-by: Shivam Mishra (cherry picked from commit cd1ab8e23c4a1de4bbd5137c34a864af53788237) Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> --- frappe/__init__.py | 4 ++++ frappe/desk/search.py | 13 +++++++++++++ frappe/tests/test_search.py | 28 ++++++++++++++++++++++++++++ requirements.txt | 3 ++- 4 files changed, 47 insertions(+), 1 deletion(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index a2276ee149..206283fd3a 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -1707,3 +1707,7 @@ def mock(type, size=1, locale='en'): from frappe.chat.util import squashify return squashify(results) + +def validate_and_sanitize_search_inputs(fn): + from frappe.desk.search import validate_and_sanitize_search_inputs as func + return func(fn) diff --git a/frappe/desk/search.py b/frappe/desk/search.py index b4b54b4b6e..798e499bb9 100644 --- a/frappe/desk/search.py +++ b/frappe/desk/search.py @@ -10,6 +10,7 @@ from frappe.handler import is_whitelisted from frappe import _ from six import string_types import re +import wrapt UNTRANSLATED_DOCTYPES = ["DocType", "Role"] @@ -206,3 +207,15 @@ def scrub_custom_query(query, key, txt): if '%s' in query: query = query.replace('%s', ((txt or '') + '%')) return query + +@wrapt.decorator +def validate_and_sanitize_search_inputs(fn, instance, args, kwargs): + kwargs.update(dict(zip(fn.__code__.co_varnames, args))) + sanitize_searchfield(kwargs['searchfield']) + kwargs['start'] = cint(kwargs['start']) + kwargs['page_len'] = cint(kwargs['page_len']) + + if kwargs['doctype'] and not frappe.db.exists('DocType', kwargs['doctype']): + return [] + + return fn(**kwargs) \ No newline at end of file diff --git a/frappe/tests/test_search.py b/frappe/tests/test_search.py index 5cab827218..192fe9dd25 100644 --- a/frappe/tests/test_search.py +++ b/frappe/tests/test_search.py @@ -50,3 +50,31 @@ class TestSearch(unittest.TestCase): def tearDown(self): frappe.local.lang = 'en' + + def test_validate_and_sanitize_search_inputs(self): + + # should raise error if searchfield is injectable + self.assertRaises(frappe.DataError, + get_data, *('User', 'Random', 'select * from tabSessions) --', '1', '10', dict())) + + # page_len and start should be converted to int + self.assertListEqual(get_data('User', 'Random', 'email', 'name or (select * from tabSessions)', '10', dict()), + ['User', 'Random', 'email', 0, 10, {}]) + self.assertListEqual(get_data('User', 'Random', 'email', page_len='2', start='10', filters=dict()), + ['User', 'Random', 'email', 10, 2, {}]) + + # DocType can be passed as None which should be accepted + self.assertListEqual(get_data(None, 'Random', 'email', '2', '10', dict()), + [None, 'Random', 'email', 2, 10, {}]) + + # return empty string if passed doctype is invalid + self.assertListEqual(get_data("Random DocType", 'Random', 'email', '2', '10', dict()), []) + + # should not fail if function is called via frappe.call with extra arguments + args = ("Random DocType", 'Random', 'email', '2', '10', dict()) + kwargs = {'as_dict': False} + self.assertListEqual(frappe.call('frappe.tests.test_search.get_data', *args, **kwargs), []) + +@frappe.validate_and_sanitize_search_inputs +def get_data(doctype, txt, searchfield, start, page_len, filters): + return [doctype, txt, searchfield, start, page_len, filters] \ No newline at end of file diff --git a/requirements.txt b/requirements.txt index 33cd292bf6..2995553541 100644 --- a/requirements.txt +++ b/requirements.txt @@ -69,4 +69,5 @@ Whoosh==2.7.4 xlrd==1.2.0 zxcvbn-python==4.4.24 pycryptodome==3.9.8 -paytmchecksum==1.7.0 \ No newline at end of file +paytmchecksum==1.7.0 +wrapt==1.10.11 \ No newline at end of file