KNOX-3409: Validate knoxauth theme name to Harden Branding Support - #1341
Open
lmccay wants to merge 1 commit into
Open
KNOX-3409: Validate knoxauth theme name to Harden Branding Support#1341lmccay wants to merge 1 commit into
lmccay wants to merge 1 commit into
Conversation
The knoxauth login page read the "theme" query parameter and wrote it into
a <link> tag via document.write() with no validation, allowing arbitrary
markup injection on the page that collects user credentials. The value was
also persisted to localStorage and replayed on later visits, so a single
malicious link kept executing on subsequent visits from a clean URL.
Theme names are now validated against ^[a-zA-Z0-9_-]{1,64}$ on every path
(URL parameter, localStorage, and the configured default), which rejects
quotes, angle brackets, dots and path separators. The stylesheet element is
built with DOM APIs instead of string concatenation, so a theme name can
never be parsed as markup. A stored value that fails validation, or that
does not resolve to an installed theme, is discarded rather than replayed.
Because the only URL the loader can produce is styles/themes/<name>/theme.css,
the themes actually installed on the server remain the effective allowlist and
no new configuration is required. The README and deployment guide are updated
to describe the validation, replacing two security claims that were inaccurate
as written.
Reported by Quinn Nguyen. Not present in any released version - the theming
feature (KNOX-3283) has only ever been on master.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Test Results40 tests 40 ✅ 6s ⏱️ Results for commit 3fe5c82. |
lmccay
requested review from
pzampino and
smolnar82
and
a lite review from Copilot
August 8, 2026 01:11
There was a problem hiding this comment.
Pull request overview
Hardens KnoxAuth login-page theming so attacker-controlled theme inputs (URL/localStorage/default) can’t inject markup or traverse paths, and updates documentation to accurately describe the security properties.
Changes:
- Validates theme names against
^[a-zA-Z0-9_-]{1,64}$for URL, localStorage, and configured defaults. - Replaces
document.write()stylesheet injection with DOM-based<link>creation. - Updates theming documentation (security considerations and CSP guidance) to match the new behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| gateway-applications/src/main/resources/applications/knoxauth/app/styles/themes/README.md | Updates security considerations to document theme-name validation and DOM-based stylesheet insertion. |
| gateway-applications/src/main/resources/applications/knoxauth/app/styles/themes/DEPLOYMENT.md | Documents validation behavior and adds CSP deployment guidance for knoxauth. |
| gateway-applications/src/main/resources/applications/knoxauth/app/login.html | Implements validation and safe stylesheet loading for KnoxAuth theme selection. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+65
to
68
| // Only a validated theme is persisted for future visits | ||
| try { | ||
| localStorage.setItem('knox-auth-theme', theme); | ||
| localStorage.setItem('knox-auth-theme', requested); | ||
| } catch (e) { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
KNOX-3409 - Validate knoxauth theme name to Harden Branding Support
What changes were proposed in this pull request?
The knoxauth login page read the "theme" query parameter and wrote it into a tag via document.write() with no validation, allowing arbitrary markup injection on the page that collects user credentials. The value was also persisted to localStorage and replayed on later visits, so a single malicious link kept executing on subsequent visits from a clean URL.
Theme names are now validated against ^[a-zA-Z0-9_-]{1,64}$ on every path (URL parameter, localStorage, and the configured default), which rejects quotes, angle brackets, dots and path separators. The stylesheet element is built with DOM APIs instead of string concatenation, so a theme name can never be parsed as markup. A stored value that fails validation, or that does not resolve to an installed theme, is discarded rather than replayed.
Because the only URL the loader can produce is styles/themes//theme.css, the themes actually installed on the server remain the effective allowlist and no new configuration is required. The README and deployment guide are updated to describe the validation, replacing two security claims that were inaccurate as written.
Reported by Quinn Nguyen. Not present in any released version - the theming feature (KNOX-3283) has only ever been on master.
How was this patch tested?
Manually tested.
There is no existing test harness for this area, we will follow up with a test suite afterward.