Security: Untrusted stylesheet URL is injected into document head#1234
Conversation
…cument The frontend loads `custom_css_url` from runtime config and directly appends it as a `<link rel="stylesheet">` without origin allowlisting or scheme validation. If an attacker can influence this config, they can inject hostile CSS to perform UI redressing, data exfiltration via CSS side channels, or phishing overlays. Affected files: AppInitialization.tsx Signed-off-by: tuanaiseo <221258316+tuanaiseo@users.noreply.github.com>
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughThe stylesheet injection logic in Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/frontend/src/components/AppInitialization.tsx`:
- Around line 22-41: The useEffect in the AppInitialization component that
injects the <link> for custom_css_url must add a cleanup to remove the injected
element (id 'meet-custom-css') when custom_css_url changes or the component
unmounts; update the effect (the function that creates the link element) to
first remove any existing element with id 'meet-custom-css' (or return a cleanup
function that removes it) so you don't leave duplicate or stale stylesheets in
the DOM, and keep the try/catch around URL parsing while ensuring the cleanup
runs even if parsing fails.
- Around line 38-39: The empty catch block in AppInitialization.tsx silently
swallows errors when loading or applying the configured custom_css_url; update
the catch to log a warning (including the caught error) when not in production
so configuration problems are visible—e.g., in the try/catch around
custom_css_url handling inside the AppInitialization component, change the catch
to accept an error parameter and call console.warn or a logger like
process.env.NODE_ENV !== 'production' && console.warn("Failed to load
custom_css_url:", error) (or use your app logger) so the message includes
context and the error object.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d07edac1-cf44-4377-b6f1-c55933748e0d
📒 Files selected for processing (1)
src/frontend/src/components/AppInitialization.tsx
| useEffect(() => { | ||
| if (custom_css_url) { | ||
| const link = document.createElement('link') | ||
| link.href = custom_css_url | ||
| link.id = 'meet-custom-css' | ||
| link.rel = 'stylesheet' | ||
| document.head.appendChild(link) | ||
| try { | ||
| const customCssUrl = new URL(custom_css_url, window.location.origin) | ||
| const trustedOrigins = [window.location.origin] | ||
|
|
||
| if ( | ||
| customCssUrl.protocol === 'https:' && | ||
| trustedOrigins.includes(customCssUrl.origin) | ||
| ) { | ||
| const link = document.createElement('link') | ||
| link.href = customCssUrl.toString() | ||
| link.id = 'meet-custom-css' | ||
| link.rel = 'stylesheet' | ||
| document.head.appendChild(link) | ||
| } | ||
| } catch { | ||
| } | ||
| } | ||
| }, [custom_css_url]) |
There was a problem hiding this comment.
Missing cleanup function in useEffect.
If custom_css_url changes or the component unmounts, the previously injected <link> element remains in the DOM. This could cause duplicate stylesheets or stale styles persisting after configuration changes.
🔧 Proposed fix to add cleanup
useEffect(() => {
+ let link: HTMLLinkElement | null = null
if (custom_css_url) {
try {
const customCssUrl = new URL(custom_css_url, window.location.origin)
const trustedOrigins = [window.location.origin]
if (
customCssUrl.protocol === 'https:' &&
trustedOrigins.includes(customCssUrl.origin)
) {
- const link = document.createElement('link')
+ link = document.createElement('link')
link.href = customCssUrl.toString()
link.id = 'meet-custom-css'
link.rel = 'stylesheet'
document.head.appendChild(link)
}
} catch {
}
}
+ return () => {
+ if (link) {
+ link.remove()
+ }
+ }
}, [custom_css_url])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| if (custom_css_url) { | |
| const link = document.createElement('link') | |
| link.href = custom_css_url | |
| link.id = 'meet-custom-css' | |
| link.rel = 'stylesheet' | |
| document.head.appendChild(link) | |
| try { | |
| const customCssUrl = new URL(custom_css_url, window.location.origin) | |
| const trustedOrigins = [window.location.origin] | |
| if ( | |
| customCssUrl.protocol === 'https:' && | |
| trustedOrigins.includes(customCssUrl.origin) | |
| ) { | |
| const link = document.createElement('link') | |
| link.href = customCssUrl.toString() | |
| link.id = 'meet-custom-css' | |
| link.rel = 'stylesheet' | |
| document.head.appendChild(link) | |
| } | |
| } catch { | |
| } | |
| } | |
| }, [custom_css_url]) | |
| useEffect(() => { | |
| let link: HTMLLinkElement | null = null | |
| if (custom_css_url) { | |
| try { | |
| const customCssUrl = new URL(custom_css_url, window.location.origin) | |
| const trustedOrigins = [window.location.origin] | |
| if ( | |
| customCssUrl.protocol === 'https:' && | |
| trustedOrigins.includes(customCssUrl.origin) | |
| ) { | |
| link = document.createElement('link') | |
| link.href = customCssUrl.toString() | |
| link.id = 'meet-custom-css' | |
| link.rel = 'stylesheet' | |
| document.head.appendChild(link) | |
| } | |
| } catch { | |
| } | |
| } | |
| return () => { | |
| if (link) { | |
| link.remove() | |
| } | |
| } | |
| }, [custom_css_url]) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/frontend/src/components/AppInitialization.tsx` around lines 22 - 41, The
useEffect in the AppInitialization component that injects the <link> for
custom_css_url must add a cleanup to remove the injected element (id
'meet-custom-css') when custom_css_url changes or the component unmounts; update
the effect (the function that creates the link element) to first remove any
existing element with id 'meet-custom-css' (or return a cleanup function that
removes it) so you don't leave duplicate or stale stylesheets in the DOM, and
keep the try/catch around URL parsing while ensuring the cleanup runs even if
parsing fails.
| } catch { | ||
| } |
There was a problem hiding this comment.
Silent error swallowing hinders debugging.
An empty catch block makes it difficult to diagnose configuration issues when an invalid custom_css_url is provided. Consider logging a warning in non-production environments.
🛠️ Suggested improvement
- } catch {
+ } catch (error) {
+ if (process.env.NODE_ENV !== 'production') {
+ console.warn('Invalid custom_css_url:', custom_css_url, error)
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch { | |
| } | |
| } catch (error) { | |
| if (process.env.NODE_ENV !== 'production') { | |
| console.warn('Invalid custom_css_url:', custom_css_url, error) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/frontend/src/components/AppInitialization.tsx` around lines 38 - 39, The
empty catch block in AppInitialization.tsx silently swallows errors when loading
or applying the configured custom_css_url; update the catch to log a warning
(including the caught error) when not in production so configuration problems
are visible—e.g., in the try/catch around custom_css_url handling inside the
AppInitialization component, change the catch to accept an error parameter and
call console.warn or a logger like process.env.NODE_ENV !== 'production' &&
console.warn("Failed to load custom_css_url:", error) (or use your app logger)
so the message includes context and the error object.
2 participants
Problem
The frontend loads
custom_css_urlfrom runtime config and directly appends it as a<link rel="stylesheet">without origin allowlisting or scheme validation. If an attacker can influence this config, they can inject hostile CSS to perform UI redressing, data exfiltration via CSS side channels, or phishing overlays.Severity:
highFile:
src/frontend/src/components/AppInitialization.tsxSolution
Validate
custom_css_urlagainst a strict allowlist of trusted origins andhttps:scheme before injecting. Reject or ignore untrusted URLs, and consider serving tenant customization styles from trusted backend storage instead of arbitrary external URLs.Changes
src/frontend/src/components/AppInitialization.tsx(modified)Testing