sha174n opened a new pull request, #40568:
URL: https://github.com/apache/superset/pull/40568

   ## Summary
   
   Adhoc column and metric expressions submitted in a chart-data request (the 
`sqlExpression` attribute) are user-controlled SQL that ends up inside a 
`literal_column(...)` call. Today's `_process_sql_expression` in 
`superset/models/helpers.py` runs `validate_adhoc_subquery` (which rejects 
sub-queries when `ALLOW_ADHOC_SUBQUERY` is off) and `sanitize_clause` (which 
rejects multi-statement / set-operation shapes), then returns. Neither step 
consults the operator's engine-keyed `DISALLOWED_SQL_FUNCTIONS` / 
`DISALLOWED_SQL_TABLES` configuration.
   
   That means a user able to submit an adhoc expression can call functions the 
operator has explicitly denylisted (`version()`, `pg_read_file(...)`, 
`query_to_xml(...)`, etc.) and have them executed at chart-render time.
   
   This change applies the same denylists immediately after `sanitize_clause` 
in `_process_sql_expression`. The gate fires at validation time, before the 
final SQL is rendered, and complements the equivalent gate at query execution.
   
   The block detects whether the caller pre-wrapped the expression with `SELECT 
...` (which `_process_select_expression` and `_process_orderby_expression` both 
do) and avoids the double-`SELECT` parse failure.
   
   ## Tests
   
   Added 4 cases in `tests/unit_tests/models/helpers_test.py`:
   
   - a denylisted function passed directly (`version()`) raises 
`SupersetDisallowedSQLFunctionException`
   - a denylisted function wrapped in an aggregate (`MAX(version())`) is still 
rejected
   - a benign aggregate over a regular column (`SUM(amount)`) passes through 
unchanged
   - when both denylists are empty for the engine, the extra parse step is 
skipped and any `sanitize_clause`-accepted SQL is allowed
   
   All 60 tests in the file pass:
   
   ```
   $ pytest tests/unit_tests/models/helpers_test.py -q
   ............................................................   [100%]
   60 passed in 1.88s
   ```
   
   ## Test plan
   
   - [ ] Maintainer review
   - [ ] CI green (pytest, mypy, pre-commit)


-- 
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]

Reply via email to