Brand spinner SVG is rendered inline and relies on regex-based sanitization
- Dominant language
- Python
- Stars
- 74.8k
- Forks
- 18.3k
- Avg merge
- 2d 5h
- Merged PRs (30d)
- 685
Description
I raised this with the security team first; they confirmed it and see it as a hardening item (admin-only theme write, non-default config) rather than an advisory, and suggested opening a public issue and PR.
`spa.html` renders the theme spinner inline with `{{ spinner_svg | safe }}` (from `brandSpinnerSvg` of the system-default theme, set in `superset/views/base.py`). The only guard before that raw-HTML sink is `sanitize_svg_content` in `superset/utils/core.py`, which is a regex denylist (its docstring: "Basic SVG protection ... trust admin input otherwise").
Regex SVG sanitization is bypassable, and this field is admin-set and system-default, so it's served to every user, including the logged-out login page. On deployments that relax CSP (e.g. `TALISMAN_ENABLED = False`), a sanitizer bypass becomes stored XSS for everyone.
Fix: render the spinner as ``. An SVG loaded via `
` is a passive image and can't run scripts, so it holds regardless of contents and doesn't rely on the sanitizer. Same approach the React `Loading` component already uses. PR incoming.
Contributor guide
Research direction
Start in spa.html and trace spinner_svg to brandSpinnerSvg in superset/views/base.py and sanitize_svg_content in superset/utils/core.py. Compare the existing React Loading component's spinner handling, then verify the login page renders the spinner as a passive image and add or run the relevant tests for the changed output.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python, react
- Domain
- frontend, security
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 38/100