Merge branch 'hotfix'

frappe/__init__.py

@@ -14,7 +14,7 @@ import os, sys, importlib, inspect, json
 from .exceptions import *
 from .utils.jinja import get_jenv, get_template, render_template, get_email_from_template
 
-__version__ = '10.1.37'
+__version__ = '10.1.38'
 __title__ = "Frappe Framework"
 
 local = Local()

frappe/desk/search.py

@@ -8,6 +8,31 @@ from frappe.utils import cstr, unique
 from frappe import _
 from six import string_types
 
+
+def sanitize_searchfield(searchfield):
+	blacklisted_keywords = ['select', 'delete', 'drop', 'update', 'case', 'and', 'or', 'like']
+
+	def _raise_exception():
+		frappe.throw(_('Invalid Search Field'), frappe.DataError)
+
+	if len(searchfield) >= 3:
+
+		# to avoid 1=1
+		if '=' in searchfield:
+			_raise_exception()
+
+		# in mysql -- is used for commenting the query
+		elif ' --' in searchfield:
+			_raise_exception()
+
+		# to avoid and, or and like
+		elif any(' {0} '.format(keyword) in searchfield.split() for keyword in blacklisted_keywords):
+			_raise_exception()
+
+		# to avoid select, delete, drop, update and case
+		elif any(keyword in searchfield.split() for keyword in blacklisted_keywords):
+			_raise_exception()
+
 # this is called by the Link Field
 @frappe.whitelist()
 def search_link(doctype, txt, query=None, filters=None, page_length=20, searchfield=None):
@@ -24,6 +49,9 @@ def search_widget(doctype, txt, query=None, searchfield=None, start=0,
 
 	meta = frappe.get_meta(doctype)
 
+	if searchfield:
+		sanitize_searchfield(searchfield)
+
 	if not searchfield:
 		searchfield = "name"
 

frappe/model/db_query.py

@@ -140,7 +140,7 @@ class DatabaseQuery(object):
 
 		if self.or_conditions:
 			args.conditions += (' or ' if args.conditions else "") + \
-				 ' or '.join(self.or_conditions)
+				' or '.join(self.or_conditions)
 
 		self.set_field_tables()
 
@@ -190,7 +190,8 @@ class DatabaseQuery(object):
 			As field contains `,` and mysql function `version()`, with the help of regex
 			the system will filter out this field.
 		'''
-		regex = re.compile('^.*[,();].*')
+
+		sub_query_regex = re.compile("^.*[,();].*")
 		blacklisted_keywords = ['select', 'create', 'insert', 'delete', 'drop', 'update', 'case']
 		blacklisted_functions = ['concat', 'concat_ws', 'if', 'ifnull', 'nullif', 'coalesce',
 			'connection_id', 'current_user', 'database', 'last_insert_id', 'session_user',
@@ -200,14 +201,22 @@ class DatabaseQuery(object):
 			frappe.throw(_('Cannot use sub-query or function in fields'), frappe.DataError)
 
 		for field in self.fields:
-			if regex.match(field):
-				if any(keyword in field.lower() for keyword in blacklisted_keywords):
+			if sub_query_regex.match(field):
+				if any(keyword in field.lower().split() for keyword in blacklisted_keywords):
 					_raise_exception()
 
-				if any("{0}(".format(keyword) in field.lower() \
-					for keyword in blacklisted_functions):
+				if any("({0}".format(keyword) in field.lower() for keyword in blacklisted_keywords):
 					_raise_exception()
 
+				if any("{0}(".format(keyword) in field.lower() for keyword in blacklisted_functions):
+					_raise_exception()
+
+			if re.compile("[a-zA-Z]+\s*'").match(field):
+				_raise_exception()
+
+			if re.compile('[a-zA-Z]+\s*,').match(field):
+				_raise_exception()
+
 	def extract_tables(self):
 		"""extract tables from fields"""
 		self.tables = ['`tab' + self.doctype + '`']

frappe/model/naming.py

@@ -202,7 +202,7 @@ def append_number_if_name_exists(doctype, value, fieldname='name', separator='-'
 	filters.update({fieldname: value})
 	exists = frappe.db.exists(doctype, filters)
 
-	regex = '^{value}{separator}[[:digit:]]$'.format(value=re.escape(value), separator=separator)
+	regex = '^{value}{separator}\d+$'.format(value=re.escape(value), separator=separator)
 	if exists:
 		last = frappe.db.sql("""select {fieldname} from `tab{doctype}`
 			where {fieldname} regexp %s

frappe/public/css/bootstrap.css

@@ -193,9 +193,9 @@ th {
   *,
   *:before,
   *:after {
-    color: #000 !important;
+    color: #000;
+    background: transparent;
     text-shadow: none !important;
-    background: transparent !important;
     -webkit-box-shadow: none !important;
             box-shadow: none !important;
   }
@@ -254,11 +254,11 @@ th {
   }
   .table td,
   .table th {
-    background-color: #fff !important;
+    background-color: #fff;
   }
   .table-bordered th,
   .table-bordered td {
-    border: 1px solid #ddd !important;
+    border: 1px solid #ddd;
   }
 }
 @font-face {

frappe/tests/test_db_query.py

@@ -117,6 +117,12 @@ class TestReportview(unittest.TestCase):
 			fields=["name", "issingle, IF(issingle=1, (SELECT name from tabUser), count(*))"],
 			limit_start=0, limit_page_length=1)
 
+		self.assertRaises(frappe.DataError, DatabaseQuery("DocType").execute,
+			fields=["name", "issingle ''"],limit_start=0, limit_page_length=1)
+
+		self.assertRaises(frappe.DataError, DatabaseQuery("DocType").execute,
+			fields=["name", "issingle,'"],limit_start=0, limit_page_length=1)
+
 		data = DatabaseQuery("DocType").execute(fields=["name", "issingle", "count(name)"],
 			limit_start=0, limit_page_length=1)
 		self.assertTrue('count(name)' in data[0])
@@ -133,6 +139,48 @@ class TestReportview(unittest.TestCase):
 			"datediff(modified, creation) as date_diff"], limit_start=0, limit_page_length=1)
 		self.assertTrue('date_diff' in data[0])
 
+	def test_filter_sanitizer(self):
+		self.assertRaises(frappe.DataError, DatabaseQuery("DocType").execute,
+				fields=["name"], filters={'istable,': 1}, limit_start=0, limit_page_length=1)
+
+		self.assertRaises(frappe.DataError, DatabaseQuery("DocType").execute,
+				fields=["name"], filters={'editable_grid,': 1}, or_filters={'istable,': 1},
+				limit_start=0, limit_page_length=1)
+
+		self.assertRaises(frappe.DataError, DatabaseQuery("DocType").execute,
+				fields=["name"], filters={'editable_grid,': 1},
+				or_filters=[['DocType', 'istable,', '=', 1]],
+				limit_start=0, limit_page_length=1)
+
+		self.assertRaises(frappe.DataError, DatabaseQuery("DocType").execute,
+				fields=["name"], filters={'editable_grid,': 1},
+				or_filters=[['DocType', 'istable', '=', 1], ['DocType', 'beta and 1=1', '=', 0]],
+				limit_start=0, limit_page_length=1)
+
+		data = DatabaseQuery("DocType").execute(fields=["name", "issingle"],
+				filters={'editable_grid': 1, 'module': 'Core'},
+				or_filters=[['DocType', 'istable', '=', 1]],
+				limit_start=0, limit_page_length=1)
+		self.assertEquals('DocField', data[0]['name'])
+
+		data = DatabaseQuery("DocType").execute(fields=["name"],
+				filters={'issingle': 1}, or_filters=[['DocType', 'module', '=', 'Core']],
+				limit_start=0, limit_page_length=1)
+		self.assertEquals('User Permission for Page and Report', data[0]['name'])
+
+		data = DatabaseQuery("DocType").execute(fields=["name"],
+				filters={'track_changes': 1, 'module': 'Core'},
+				limit_start=0, limit_page_length=4)
+		self.assertEquals(['File', 'Version', 'Data Import', 'Domain Settings'],
+			[d['name'] for d in data])
+
+		data = DatabaseQuery("DocType").execute(fields=["name"],
+				filters=[['DocType', 'ifnull(track_changes, 0)', '=', 0],
+					['DocType', 'module', '=', 'Core']],
+				limit_start=0, limit_page_length=4)
+		self.assertEquals(['DocField', 'User Permission for Page and Report', 'SMS Settings', 'Block Module'],
+			[d['name'] for d in data])
+
 def create_event(subject="_Test Event", starts_on=None):
 	""" create a test event """
 
@@ -145,4 +193,4 @@ def create_event(subject="_Test Event", starts_on=None):
 		"starts_on": get_datetime(starts_on),
 	}).insert(ignore_permissions=True)
 
-	return event
+	return event

frappe/tests/test_search.py

@@ -0,0 +1,28 @@
+# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
+# MIT License. See license.txt
+
+from __future__ import unicode_literals
+import unittest
+import frappe
+from frappe.desk.search import search_link
+
+
+class TestSearch(unittest.TestCase):
+	def test_search_field_sanitizer(self):
+		# pass
+		search_link('DocType', 'User', query=None, filters=None, page_length=20, searchfield='name')
+		result = frappe.response['results'][0]
+		self.assertTrue('User' in result['value'])
+
+		#raise exception on injection
+		self.assertRaises(frappe.DataError,
+			search_link, 'DocType', 'Customer', query=None, filters=None,
+			page_length=20, searchfield='1=1')
+
+		self.assertRaises(frappe.DataError,
+			search_link, 'DocType', 'Customer', query=None, filters=None,
+			page_length=20, searchfield='select * from tabSessions) --')
+
+		self.assertRaises(frappe.DataError,
+			search_link, 'DocType', 'Customer', query=None, filters=None,
+			page_length=20, searchfield='name or (select * from tabSessions)')

frappe/utils/data.py

@@ -801,6 +801,8 @@ def get_filter(doctype, f):
 
 	f = frappe._dict(doctype=f[0], fieldname=f[1], operator=f[2], value=f[3])
 
+	sanitize_column(f.fieldname)
+
 	if not f.operator:
 		# if operator is missing
 		f.operator = "="
@@ -840,6 +842,27 @@ def make_filter_dict(filters):
 
 	return _filter
 
+def sanitize_column(column_name):
+	from frappe import _
+	regex = re.compile("^.*[,'();].*")
+	blacklisted_keywords = ['select', 'create', 'insert', 'delete', 'drop', 'update', 'case', 'and', 'or']
+
+	def _raise_exception():
+		frappe.throw(_("Invalid field name {0}").format(column_name), frappe.DataError)
+
+	if 'ifnull' in column_name:
+		if regex.match(column_name):
+			# to avoid and, or
+			if any(' {0} '.format(keyword) in column_name.split() for keyword in blacklisted_keywords):
+				_raise_exception()
+
+			# to avoid select, delete, drop, update and case
+			elif any(keyword in column_name.split() for keyword in blacklisted_keywords):
+				_raise_exception()
+
+	elif regex.match(column_name):
+		_raise_exception()
+
 def scrub_urls(html):
 	html = expand_relative_urls(html)
 	# encoding should be responsibility of the composer

frappe/utils/global_search.py

@@ -8,9 +8,9 @@ import re
 import redis
 from frappe.utils import cint, strip_html_tags
 from frappe.model.base_document import get_controller
+from frappe.model.db_schema import varchar_len
 from six import text_type
 
-
 def setup_global_search_table():
 	"""
 	Creates __global_seach table
@@ -19,17 +19,16 @@ def setup_global_search_table():
 	if not '__global_search' in frappe.db.get_tables():
 		frappe.db.sql('''create table __global_search(
 			doctype varchar(100),
-			name varchar(140),
-			title varchar(140),
+			name varchar({varchar_len}),
+			title varchar({varchar_len}),
 			content text,
 			fulltext(content),
-			route varchar(140),
+			route varchar({varchar_len}),
 			published int(1) not null default 0,
 			unique `doctype_name` (doctype, name))
 			COLLATE=utf8mb4_unicode_ci
 			ENGINE=MyISAM
-			CHARACTER SET=utf8mb4''')
-
+			CHARACTER SET=utf8mb4'''.format(varchar_len=varchar_len))
 
 def reset():
 	"""
@@ -142,8 +141,8 @@ def rebuild_for_doctype(doctype):
 				"name": frappe.db.escape(doc.name),
 				"content": frappe.db.escape(' ||| '.join(content or '')),
 				"published": published,
-				"title": frappe.db.escape(title or ''),
-				"route": frappe.db.escape(route or '')
+				"title": frappe.db.escape(title or '')[:int(varchar_len)],
+				"route": frappe.db.escape(route or '')[:int(varchar_len)]
 			})
 	if all_contents:
 		insert_values_for_multiple_docs(all_contents)
@@ -259,7 +258,8 @@ def update_global_search(doc):
 
 		frappe.flags.update_global_search.append(
 			dict(doctype=doc.doctype, name=doc.name, content=' ||| '.join(content or ''),
-				published=published, title=doc.get_title(), route=doc.get('route')))
+				published=published, title=doc.get_title()[:int(varchar_len)], route=doc.get('route')))
+
 		enqueue_global_search()