From 3fe5c82f8446e55533b402b97038eed3a975740e Mon Sep 17 00:00:00 2001 From: Larry McCay Date: Fri, 7 Aug 2026 20:40:56 -0400 Subject: [PATCH 1/2] KNOX-3409: Validate knoxauth theme name to prevent DOM-based XSS 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. Co-Authored-By: Claude Opus 5 (1M context) --- .../applications/knoxauth/app/login.html | 69 +++++++++++++++---- .../knoxauth/app/styles/themes/DEPLOYMENT.md | 18 ++++- .../knoxauth/app/styles/themes/README.md | 15 +++- 3 files changed, 85 insertions(+), 17 deletions(-) diff --git a/gateway-applications/src/main/resources/applications/knoxauth/app/login.html b/gateway-applications/src/main/resources/applications/knoxauth/app/login.html index 8c69ec9c20..ec27cebb6c 100644 --- a/gateway-applications/src/main/resources/applications/knoxauth/app/login.html +++ b/gateway-applications/src/main/resources/applications/knoxauth/app/login.html @@ -36,40 +36,83 @@ diff --git a/gateway-applications/src/main/resources/applications/knoxauth/app/styles/themes/DEPLOYMENT.md b/gateway-applications/src/main/resources/applications/knoxauth/app/styles/themes/DEPLOYMENT.md index 220c3177a6..e746fdd331 100644 --- a/gateway-applications/src/main/resources/applications/knoxauth/app/styles/themes/DEPLOYMENT.md +++ b/gateway-applications/src/main/resources/applications/knoxauth/app/styles/themes/DEPLOYMENT.md @@ -332,7 +332,18 @@ Organizations with branding requirements or compliance needs should use `KNOX_TH ### Theme Name Validation -The theme loader only loads files from `styles/themes/THEME_NAME/theme.css`. Path traversal attacks (e.g., `?theme=../../etc/passwd`) are prevented by the URL structure. +The `?theme=` parameter and the saved localStorage preference are attacker-influenceable, +so the theme loader validates every candidate against `^[a-zA-Z0-9_-]{1,64}$` before it is +stored or used. This rejects quotes, angle brackets, dots and path separators, which +blocks both markup injection and path traversal (e.g. `?theme=../../etc/passwd`). +Validation is applied to values read back from localStorage as well as to the URL +parameter, so a value saved by an earlier visit cannot bypass it, and the stylesheet +element is built with DOM APIs rather than string concatenation. + +Because the only URL the loader can produce is `styles/themes/THEME_NAME/theme.css`, the +themes actually installed on the server are the effective allowlist. A name that does not +match an installed theme fails to load, the base Knox styles remain in effect, and the +saved preference is discarded. ### Content Security Policy @@ -340,6 +351,11 @@ If you have strict CSP, ensure it allows: - Loading CSS from same origin - Loading fonts from Google Fonts (if using modern theme) +A policy can be applied to the knoxauth route with the WebAppSec provider's +`SecurityHeaderFilter`, which emits arbitrary response headers from its init +parameters. Note that `login.html` currently uses inline scripts and inline event +handlers, so a policy for this page needs `'unsafe-inline'` for `script-src`. + Example CSP: ``` Content-Security-Policy: style-src 'self' https://fonts.googleapis.com; diff --git a/gateway-applications/src/main/resources/applications/knoxauth/app/styles/themes/README.md b/gateway-applications/src/main/resources/applications/knoxauth/app/styles/themes/README.md index e15aa77214..4bbfd58725 100644 --- a/gateway-applications/src/main/resources/applications/knoxauth/app/styles/themes/README.md +++ b/gateway-applications/src/main/resources/applications/knoxauth/app/styles/themes/README.md @@ -441,9 +441,18 @@ Modern CSS features used: ## Security Considerations -1. **XSS Protection**: Theme names are not executed as code, only used to construct file paths -2. **Path Traversal**: Theme loader only loads files from `styles/themes/` directory -3. **Content Security Policy**: Ensure CSP allows loading external fonts if using Google Fonts +1. **Theme Name Validation**: Theme names arrive from untrusted sources (the `?theme=` + URL parameter and the saved localStorage preference), so each candidate must match + `^[a-zA-Z0-9_-]{1,64}$` before it is stored or used. Validation is applied on the + localStorage read path as well as the URL, and a value that fails is discarded. +2. **XSS Protection**: The stylesheet element is created with DOM APIs + (`document.createElement`) rather than by concatenating markup, so a theme name can + never be parsed as HTML. +3. **Path Traversal**: The validation pattern rejects dots and path separators, so the + only URL the loader can produce is `styles/themes/THEME_NAME/theme.css`. A name that + does not correspond to an installed theme simply fails to load and the base styles + remain in effect - the themes present on the server are the effective allowlist. +4. **Content Security Policy**: Ensure CSP allows loading external fonts if using Google Fonts ## Troubleshooting From e956db5c1d9b245582809be8f2a867d88a0c7338 Mon Sep 17 00:00:00 2001 From: Larry McCay Date: Sat, 8 Aug 2026 13:12:44 -0400 Subject: [PATCH 2/2] KNOX-3409: Discard an unresolvable theme preference on the same visit Addresses review feedback. The cleanup flag tracked whether the theme had been read from localStorage rather than whether it is currently held there. A theme supplied via ?theme= is persisted immediately, so it is equally eligible for cleanup, but the flag stayed false and no onerror handler was attached. A name that passed validation without resolving to an installed theme was therefore saved and replayed once before being cleared. The flag is renamed to themeIsPersisted and set after a successful setItem, so it reflects the invariant the cleanup actually depends on. Setting it inside the try means a failed write - localStorage disabled - correctly leaves nothing to clean up. Behaviour for an admin-configured theme is unchanged: a deployment error does not discard a user preference. Co-Authored-By: Claude Opus 5 (1M context) --- .../applications/knoxauth/app/login.html | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/gateway-applications/src/main/resources/applications/knoxauth/app/login.html b/gateway-applications/src/main/resources/applications/knoxauth/app/login.html index ec27cebb6c..6a30f61065 100644 --- a/gateway-applications/src/main/resources/applications/knoxauth/app/login.html +++ b/gateway-applications/src/main/resources/applications/knoxauth/app/login.html @@ -50,7 +50,9 @@ // Load theme based on: deployment lock > URL parameter > localStorage > deployment default var theme; - var fromSavedPreference = false; + // True when the theme about to load is also the value held in localStorage, + // which is what makes it eligible for cleanup if the stylesheet fails. + var themeIsPersisted = false; // Check if theme is locked (admin-enforced theme) if (typeof KNOX_THEME_LOCKED !== 'undefined' && KNOX_THEME_LOCKED === true) { @@ -65,6 +67,7 @@ // Only a validated theme is persisted for future visits try { localStorage.setItem('knox-auth-theme', requested); + themeIsPersisted = true; } catch (e) { // LocalStorage may be disabled, continue without saving } @@ -79,7 +82,7 @@ if (saved && !theme) { localStorage.removeItem('knox-auth-theme'); } - fromSavedPreference = theme !== null; + themeIsPersisted = theme !== null; } catch (e) { // LocalStorage may be disabled, use deployment default theme = null; @@ -97,12 +100,12 @@ link.rel = 'stylesheet'; link.type = 'text/css'; link.id = 'knox-theme'; - // The server is the authority on which themes exist. If a theme taken - // from the saved preference fails to load, it is not installed here, - // so forget it rather than retrying it on every visit. A failure of an - // admin-configured theme is left alone - that is a deployment problem + // The server is the authority on which themes exist. If the theme held + // in localStorage fails to load, it is not installed here, so forget it + // straight away rather than replaying it on the next visit. A failure of + // an admin-configured theme is left alone - that is a deployment problem // to fix, not the user's preference to discard. - if (fromSavedPreference) { + if (themeIsPersisted) { link.onerror = function() { try { localStorage.removeItem('knox-auth-theme');