Abdulrehman-PIAIC80387 opened a new pull request, #41877:
URL: https://github.com/apache/superset/pull/41877
### SUMMARY
Fixes #41869.
Two related bugs in `PrestoBaseEngineSpec` allowed authenticated SQL Lab
users on Presto/Trino databases with template processing enabled to inject
arbitrary SQL via the `{{ presto.latest_sub_partition(...) }}` Jinja macro:
1. **Broken kwarg guard** in `latest_sub_partition` (line 675) — the check
`if k not in k in part_fields:` is a chained comparison that Python evaluates
as `(k not in k) and (k in part_fields)`. Since `k not in k` is always `False`
for any string, the guard was unreachable and any caller-supplied kwarg name
flowed through. The `pylint: disable=comparison-with-itself` comment shows the
linter had flagged this — the warning was suppressed rather than fixed.
2. **Raw f-string interpolation** in `_partition_query` (line 501) — filter
values were interpolated into the WHERE clause without escaping, so a `'` in
the value broke out of the SQL string literal and let injected SQL execute
against the database connection's service account.
Both bugs are fixed and covered by regression tests. `TrinoEngineSpec` is
also fixed because its `latest_sub_partition` delegates to
`super().latest_sub_partition(...)` and it inherits `_partition_query`
unchanged. `HiveEngineSpec` is unaffected — its `latest_sub_partition` is a
no-op and its `_partition_query` does not interpolate filter values.
### TESTING INSTRUCTIONS
```bash
pytest tests/unit_tests/db_engine_specs/test_presto.py -x
pytest tests/unit_tests/db_engine_specs/test_trino.py -x
```
Two new regression tests were added to `test_presto.py`:
- `test_latest_sub_partition_rejects_unknown_field` — asserts
`SupersetTemplateException` is now raised when a kwarg name is not in the
partitioning key. Would have failed against the broken chained-comparison guard.
- `test_partition_query_escapes_single_quote_in_filter_value` — asserts that
a `'` in a filter value is doubled (`''`) so injected SQL stays inside the
string literal.
Local verification: 44/44 `test_presto.py` tests pass, 96/96 `test_trino.py`
tests pass.
### ADDITIONAL INFORMATION
- [x] Has associated issue: Fixes #41869
- [x] Required feature flags: none
- [x] Changes UI: no
- [x] Includes DB Migration: no
- [x] Includes CLI or Node.js commands: no
- [x] Breaking change: no
--
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]
