Merge pull request #33344 from akhilnarang/fix-orderby-groupby

fix(db_query): improve order_by/group_by sanitization

frappe/model/db_query.py

@@ -1117,20 +1117,33 @@ from {tables}
 		if not parameters:
 			return
 
-		blacklisted_sql_functions = {
-			"sleep",
-		}
 		_lower = parameters.lower()
 
-		if "select" in _lower and "from" in _lower:
-			frappe.throw(_("Cannot use sub-query in order by"))
-
 		if ORDER_GROUP_PATTERN.match(_lower):
 			frappe.throw(_("Illegal SQL Query"))
 
+		subquery_indicators = {
+			r"union",
+			r"intersect",
+			r"select\b.*\bfrom",
+		}
+
+		if any(re.search("\b" + pattern + "\b", _lower) for pattern in subquery_indicators):
+			frappe.throw(_("Cannot use sub-query here."))
+
+		blacklisted_sql_functions = {
+			"sleep",
+			"benchmark",
+			"extractvalue",
+			"database",
+			"user",
+			"current_user",
+			"version",
+			"substr",
+			"substring",
+		}
+
 		for field in parameters.split(","):
-			if field.count('"') % 2 or field.count("'") % 2 or field.count("`") % 2:
-				frappe.throw(_("Invalid field name: {0}").format(field))
 			field = field.strip()
 			full_field_name = "." in field and field.startswith("`tab")
 
@@ -1143,7 +1156,7 @@ from {tables}
 
 			# Check for SQL function using regex with word boundaries and optional whitespace before parenthesis
 			for func in blacklisted_sql_functions:
-				if re.search(r"\b" + re.escape(func) + r"\s*\(", field.lower()):
+				if re.search(r"\b" + re.escape(func) + r"\W*\(", field.lower()):
 					frappe.throw(_("Cannot use {0} in order/group by").format(field))
 
 	def add_limit(self):