fix(dashboard): URL-encode native_filters in permalink redirect#40660
fix(dashboard): URL-encode native_filters in permalink redirect#40660rusackas wants to merge 1 commit into
Conversation
dashboard_permalink concatenated the native_filters param value into the redirect URL without URL-encoding (every other param went through parse.urlencode). A permalink whose stored native_filters value contained '&'/'#'/'=' could inject additional query parameters into the redirect target, modifying the dashboard filter state shown to a victim. Encode all param values uniformly; Flask decodes them back on read, so legitimate native_filters values are unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Code Review Agent Run #5addcdActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #40660 +/- ##
==========================================
- Coverage 64.18% 63.81% -0.38%
==========================================
Files 2591 2651 +60
Lines 138471 142151 +3680
Branches 32120 32565 +445
==========================================
+ Hits 88883 90712 +1829
- Misses 48056 49876 +1820
- Partials 1532 1563 +31
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR hardens the dashboard_permalink redirect in Superset’s backend (superset/views/core.py) by ensuring permalink-provided dashboard URL parameters are safely URL-encoded, preventing query-parameter injection via specially crafted native_filters values.
Changes:
- Removed the
native_filtersspecial-case concatenation indashboard_permalink. - URL-encode all
state.urlParamsentries uniformly usingurllib.parse.urlencode. - Added clarifying inline comments explaining the injection risk and decoding behavior.
| if url_params := state.get("urlParams"): | ||
| for param_key, param_val in url_params: | ||
| if param_key == "native_filters": | ||
| # native_filters doesnt need to be encoded here | ||
| url = f"{url}&native_filters={param_val}" | ||
| else: | ||
| params = parse.urlencode([(param_key, param_val)]) | ||
| url = f"{url}&{params}" | ||
| # URL-encode every param value (including native_filters) so a | ||
| # value containing '&'/'#'/'=' cannot inject extra parameters | ||
| # into the redirect target. Flask decodes the value back on read. | ||
| params = parse.urlencode([(param_key, param_val)]) | ||
| url = f"{url}&{params}" |
There was a problem hiding this comment.
Good call on the test coverage gap. The existing integration test suite for permalink redirects is in tests/integration_tests/core_tests.py. Adding a parametrized test covering reserved characters in native_filters values is a worthwhile follow-up, but out of scope for this focused fix. Will track separately.
|
The provided |
SUMMARY
dashboard_permalink(superset/views/core.py) concatenated thenative_filtersURL-param value into the redirect URL without URL-encoding, while every other param went throughparse.urlencode. The code even had a comment "native_filters doesnt need to be encoded here." A permalink whose storednative_filtersvalue contained&/#/=could therefore inject additional query parameters into the redirect target, altering the dashboard filter state shown to a victim who opens the permalink (ASVS 1.2.2).Fix: encode all param values uniformly via
parse.urlencode. Flask URL-decodes them back when readingrequest.args, so legitimatenative_filtersvalues render identically — the encoding is transparent for valid input and only neutralizes injection.TESTING INSTRUCTIONS
A permalink whose
native_filtersvalue contains special characters now appears percent-encoded in the redirectLocation(no extra params injected).ADDITIONAL INFORMATION
🤖 Generated with Claude Code