From 081b17fbe89f49acf6bd33a96d6845f1b9160eb7 Mon Sep 17 00:00:00 2001
From: Saurabh <saurabh6790@gmail.com>
Date: Wed, 11 Jul 2018 11:04:51 +0530
Subject: [PATCH] =?UTF-8?q?[security][fix]=20tighten=20criteria=20to=20pre?=
 =?UTF-8?q?vent=20sql=20injection=20in=20search-f=E2=80=A6=20(#5800)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* [security][fix] tighten criteria to prevent sql injection in search-fields

* test cases
---
 frappe/desk/search.py       | 24 ++++++++++++++++++------
 frappe/tests/test_search.py | 12 ++++++++++++
 2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/frappe/desk/search.py b/frappe/desk/search.py
index 67e52be218..f0c47316f0 100644
--- a/frappe/desk/search.py
+++ b/frappe/desk/search.py
@@ -7,31 +7,43 @@ import frappe, json
 from frappe.utils import cstr, unique
 from frappe import _
 from six import string_types
+import re
 
 
 def sanitize_searchfield(searchfield):
 	blacklisted_keywords = ['select', 'delete', 'drop', 'update', 'case', 'and', 'or', 'like']
 
-	def _raise_exception():
-		frappe.throw(_('Invalid Search Field'), frappe.DataError)
+	def _raise_exception(searchfield):
+		frappe.throw(_('Invalid Search Field {0}').format(searchfield), frappe.DataError)
+
+	if len(searchfield) == 1:
+		# do not allow special characters to pass as searchfields
+		regex = re.compile('^.*[=;*,\'"$\-+%#@()_].*')
+		if regex.match(searchfield):
+			_raise_exception(searchfield)
 
 	if len(searchfield) >= 3:
 
 		# to avoid 1=1
 		if '=' in searchfield:
-			_raise_exception()
+			_raise_exception(searchfield)
 
 		# in mysql -- is used for commenting the query
 		elif ' --' in searchfield:
-			_raise_exception()
+			_raise_exception(searchfield)
 
 		# to avoid and, or and like
 		elif any(' {0} '.format(keyword) in searchfield.split() for keyword in blacklisted_keywords):
-			_raise_exception()
+			_raise_exception(searchfield)
 
 		# to avoid select, delete, drop, update and case
 		elif any(keyword in searchfield.split() for keyword in blacklisted_keywords):
-			_raise_exception()
+			_raise_exception(searchfield)
+
+		else:
+			regex = re.compile('^.*[=;*,\'"$\-+%#@()].*')
+			if any(regex.match(f) for f in searchfield.split()):
+				_raise_exception(searchfield)
 
 # this is called by the Link Field
 @frappe.whitelist()
diff --git a/frappe/tests/test_search.py b/frappe/tests/test_search.py
index ee4dd3cc53..9f1891753e 100644
--- a/frappe/tests/test_search.py
+++ b/frappe/tests/test_search.py
@@ -26,3 +26,15 @@ class TestSearch(unittest.TestCase):
 		self.assertRaises(frappe.DataError,
 			search_link, 'DocType', 'Customer', query=None, filters=None,
 			page_length=20, searchfield='name or (select * from tabSessions)')
+
+		self.assertRaises(frappe.DataError,
+			search_link, 'DocType', 'Customer', query=None, filters=None,
+			page_length=20, searchfield='*')
+
+		self.assertRaises(frappe.DataError,
+			search_link, 'DocType', 'Customer', query=None, filters=None,
+			page_length=20, searchfield=';')
+
+		self.assertRaises(frappe.DataError,
+			search_link, 'DocType', 'Customer', query=None, filters=None,
+			page_length=20, searchfield=';')