sha174n opened a new pull request, #40566: URL: https://github.com/apache/superset/pull/40566
### SUMMARY `is_safe_redirect_url()` (in `superset/utils/link_redirect.py`) checked for a leading `//` against the unprocessed URL string. Browsers, per the WHATWG URL parser specification, strip TAB (U+0009), LF (U+000A), and CR (U+000D) from URL strings before parsing them, including the percent-encoded forms `%09`, `%0A`, `%0D` when those appear in a Location header. A path like `/%09///host` therefore passes the literal-`//`-prefix check on the server side but the browser navigates to `//host`, treating it as a protocol-relative URL. This change normalizes the input the same way the browser will, dropping TAB/LF/CR characters and their percent-encoded forms before the `//` guard runs. ### Changes - `superset/utils/link_redirect.py`: add a module-level `_URL_STRIPPED_CONTROL_CHARS` regex matching literal and percent-encoded TAB/LF/CR; apply it to the input inside `is_safe_redirect_url()` immediately after `strip()`, before the protocol-relative guard. - `tests/unit_tests/utils/test_link_redirect.py`: extend the coverage with a parametrized case for the 10 variants browsers normalize away (literal vs percent-encoded TAB/LF/CR, single vs repeated), plus a positive test verifying that a relative URL with a tab in a query parameter still resolves as safe. ### Behaviour after this change | Input | Before | After | |---|---|---| | `/dashboard/1` | safe | safe (unchanged) | | `https://superset.example.com/x` | safe (matching host) | safe (unchanged) | | `//evil.com/x` | unsafe | unsafe (unchanged) | | `/%09///evil.com` | safe (the bug) | unsafe | | `/\t///evil.com` (literal tab) | safe (the bug) | unsafe | | `/%0A///evil.com`, `/%0D///evil.com`, etc. | safe | unsafe | ### Testing instructions ``` cd superset pytest tests/unit_tests/utils/test_link_redirect.py -v ``` 28 tests pass (16 existing + 12 new). Lints/formatters clean via pre-commit. ### Additional information - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration - [ ] Introduces new feature or API - [ ] Removes existing feature or API -- 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]
