msyavuz commented on code in PR #43111:
URL: https://github.com/apache/superset/pull/43111#discussion_r3795201921
##########
superset/security/manager.py:
##########
@@ -1107,6 +1107,76 @@ def _orderby_modified(
return False
+def _collect_allowed_sql_extras(
+ stored_chart: "Slice",
+ stored_query_context: Optional[dict[str, Any]],
+) -> tuple[set[str], set[str]]:
+ """
+ Collect the ``extras.where`` and ``extras.having`` values that a guest user
+ is allowed to send, derived from the stored chart and its query context.
+ """
+ from superset.common.form_data_query_context import freeform_where_having
+
+ allowed_where: set[str] = set()
+ allowed_having: set[str] = set()
+
+ stored_extras = freeform_where_having(stored_chart.params_dict)
+ if stored_extras.get("where"):
+ allowed_where.add(stored_extras["where"])
+ if stored_extras.get("having"):
+ allowed_having.add(stored_extras["having"])
+
+ if stored_query_context:
+ for query in stored_query_context.get("queries") or []:
+ extras = query.get("extras") or {}
+ if extras.get("where"):
+ allowed_where.add(extras["where"])
+ if extras.get("having"):
+ allowed_having.add(extras["having"])
+
+ return allowed_where, allowed_having
+
+
+def _sql_filters_modified(
+ query_context: "QueryContext",
+ form_data: dict[str, Any],
+ stored_chart: "Slice",
+ stored_query_context: Optional[dict[str, Any]],
+) -> bool:
+ """
+ Whether the request injects custom SQL predicates (``extras.where``,
+ ``extras.having``, or SQL-type adhoc filters) that are not present on the
+ stored chart.
+
+ Structured ``{col, op, val}`` filters are intentionally not constrained
+ here: they cannot carry arbitrary SQL and are legitimately added by
+ dashboard native filters, cross-filters, and drill interactions.
+ """
+ allowed_where, allowed_having = _collect_allowed_sql_extras(
+ stored_chart, stored_query_context
+ )
+
+ for query in query_context.queries:
+ extras = query.extras or {}
+ if extras.get("where") and extras["where"] not in allowed_where:
Review Comment:
A native Select filter with "Filter value is required" appends
`{expressionType: SQL, sqlExpression: '1 = 0'}`
(`superset-frontend/src/filters/utils.ts`), which becomes `extras.where` and is
never in `allowed_where` — does that 403 every in-scope chart until a value is
picked?
##########
superset/security/manager.py:
##########
@@ -1107,6 +1107,76 @@ def _orderby_modified(
return False
+def _collect_allowed_sql_extras(
+ stored_chart: "Slice",
+ stored_query_context: Optional[dict[str, Any]],
+) -> tuple[set[str], set[str]]:
+ """
+ Collect the ``extras.where`` and ``extras.having`` values that a guest user
+ is allowed to send, derived from the stored chart and its query context.
+ """
+ from superset.common.form_data_query_context import freeform_where_having
+
+ allowed_where: set[str] = set()
+ allowed_having: set[str] = set()
+
+ stored_extras = freeform_where_having(stored_chart.params_dict)
+ if stored_extras.get("where"):
+ allowed_where.add(stored_extras["where"])
+ if stored_extras.get("having"):
+ allowed_having.add(stored_extras["having"])
+
+ if stored_query_context:
+ for query in stored_query_context.get("queries") or []:
+ extras = query.get("extras") or {}
+ if extras.get("where"):
+ allowed_where.add(extras["where"])
+ if extras.get("having"):
+ allowed_having.add(extras["having"])
+
+ return allowed_where, allowed_having
+
+
+def _sql_filters_modified(
+ query_context: "QueryContext",
+ form_data: dict[str, Any],
+ stored_chart: "Slice",
+ stored_query_context: Optional[dict[str, Any]],
+) -> bool:
+ """
+ Whether the request injects custom SQL predicates (``extras.where``,
+ ``extras.having``, or SQL-type adhoc filters) that are not present on the
+ stored chart.
+
+ Structured ``{col, op, val}`` filters are intentionally not constrained
+ here: they cannot carry arbitrary SQL and are legitimately added by
Review Comment:
Structured filters can carry arbitrary SQL: `ChartDataFilterSchema.col` is
`fields.Raw`, so `{"col": {"expressionType": "SQL", "sqlExpression": "...",
"label": "x"}, "op": "!=", "val": "z"}` reaches `adhoc_column_to_sqla` and
lands in the WHERE clause unchecked.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
