Merge pull request #15907 from ankush/qb_non_select

fix: executing non-select qb code from whitelisted methods

frappe/core/doctype/server_script/test_server_script.py

@@ -139,3 +139,42 @@ class TestServerScript(unittest.TestCase):
 
 		server_script.disabled = 1
 		server_script.save()
+
+	def test_restricted_qb(self):
+		todo = frappe.get_doc(doctype="ToDo", description="QbScriptTestNote")
+		todo.insert()
+
+		script = frappe.get_doc(
+			doctype='Server Script',
+			name='test_qb_restrictions',
+			script_type = 'API',
+			api_method = 'test_qb_restrictions',
+			allow_guest = 1,
+			# whitelisted update
+			script = f'''
+frappe.db.set_value("ToDo", "{todo.name}", "description", "safe")
+'''
+		)
+		script.insert()
+		script.execute_method()
+
+		todo.reload()
+		self.assertEqual(todo.description, "safe")
+
+		# unsafe update
+		script.script = f"""
+todo = frappe.qb.DocType("ToDo")
+frappe.qb.update(todo).set(todo.description, "unsafe").where(todo.name == "{todo.name}").run()
+"""
+		script.save()
+		self.assertRaises(frappe.PermissionError, script.execute_method)
+		todo.reload()
+		self.assertEqual(todo.description, "safe")
+
+		# safe select
+		script.script = f"""
+todo = frappe.qb.DocType("ToDo")
+frappe.qb.from_(todo).select(todo.name).where(todo.name == "{todo.name}").run()
+"""
+		script.save()
+		script.execute_method()

frappe/query_builder/utils.py

@@ -59,10 +59,28 @@ def patch_query_execute():
 		return frappe.db.sql(query, params, *args, **kwargs) # nosemgrep
 
 	def prepare_query(query):
+		import inspect
+
 		param_collector = NamedParameterWrapper()
 		query = query.get_sql(param_wrapper=param_collector)
 		if frappe.flags.in_safe_exec and not query.lower().strip().startswith("select"):
-			raise frappe.PermissionError('Only SELECT SQL allowed in scripting')
+			callstack = inspect.stack()
+			if len(callstack) >= 3 and ".py" in callstack[2].filename:
+				# ignore any query builder methods called from python files
+				# assumption is that those functions are whitelisted already.
+
+				# since query objects are patched everywhere any query.run()
+				# will have callstack like this:
+				# frame0: this function prepare_query()
+				# frame1: execute_query()
+				# frame2: frame that called `query.run()`
+				#
+				# if frame2 is server script it wont have a filename and hence
+				# it shouldn't be allowed.
+				# ps. stack() returns `"<unknown>"` as filename.
+				pass
+			else:
+				raise frappe.PermissionError('Only SELECT SQL allowed in scripting')
 		return query, param_collector.get_parameters()
 
 	query_class = get_attr(str(frappe.qb).split("'")[1])