Sanitize image url from DOM node#5779
Sanitize image url from DOM node#5779christopherholland-workday wants to merge 3 commits intomainfrom
Conversation
Summary of ChangesHello @christopherholland-workday, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the application's security by implementing robust image URL sanitization. A new utility function has been added to validate image sources, ensuring that only safe Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| }} | ||
| alt={assistantName} | ||
| src={assistantIcon} | ||
| src={sanitizeImageUrl(assistantIcon)} |
Check failure
Code scanning / CodeQL
DOM text reinterpreted as HTML High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 days ago
In general, to fix this kind of issue you must ensure that untrusted text taken from the DOM is not reinterpreted as HTML or otherwise executed. For URLs, this means validating against a strict allowlist (e.g., schemes, hosts, and path patterns) and never returning raw user input unless it satisfies those constraints. For React, you should continue relying on React’s automatic attribute escaping and avoid any APIs that bypass it (like dangerouslySetInnerHTML).
Here, the best targeted fix is to strengthen sanitizeImageUrl so that it does not simply accept any HTTP(S) URL, but instead only allows URLs from the trusted avatar service already in use (DiceBear) and falls back to a known-safe default in all other cases. This both satisfies the static analysis by preventing arbitrary tainted data from flowing to the img src, and keeps the intended functionality (letting the user customize a DiceBear-based avatar) intact.
Concretely:
- Edit the
sanitizeImageUrlfunction (lines 128–145) inpackages/ui/src/views/assistants/openai/AssistantDialog.jsx. - After parsing the URL, check that:
- Protocol is
http:orhttps:(as already done), and - Hostname is exactly
api.dicebear.com, and - Path starts with
/7.x/bottts/svg.
- Protocol is
- Only if all those checks pass, return the normalized URL string (e.g.,
parsed.toString()orparsed.href); otherwise, return the fallback URL. - Do not alter the rest of the component logic;
assistantIconcan still be edited, but any non-DiceBear URL will render using the fallback, eliminating the tainted-flow concern.
No new imports are required; this uses only the standard URL class and existing variables.
| @@ -125,7 +125,7 @@ | ||
| const customization = useSelector((state) => state.customization) | ||
| const dialogRef = useRef() | ||
|
|
||
| // Sanitize image URL to prevent XSS attacks via javascript:, data:, or blob: schemes | ||
| // Sanitize image URL to prevent XSS attacks and limit to trusted avatar host | ||
| const sanitizeImageUrl = (url) => { | ||
| const fallbackUrl = `https://api.dicebear.com/7.x/bottts/svg?seed=fallback` | ||
| if (!url || typeof url !== 'string') { | ||
| @@ -133,14 +133,18 @@ | ||
| } | ||
| try { | ||
| const parsed = new URL(url, window.location.origin) | ||
| // Only allow http and https protocols | ||
| if (parsed.protocol === 'http:' || parsed.protocol === 'https:') { | ||
| return url | ||
| // Only allow http and https protocols and restrict to DiceBear bottts avatars | ||
| const isHttp = parsed.protocol === 'http:' || parsed.protocol === 'https:' | ||
| const isDiceBearHost = parsed.hostname === 'api.dicebear.com' | ||
| const isBotttsPath = parsed.pathname.startsWith('/7.x/bottts/svg') | ||
| if (isHttp && isDiceBearHost && isBotttsPath) { | ||
| // Return the normalized, safe URL string | ||
| return parsed.toString() | ||
| } | ||
| } catch (e) { | ||
| // Invalid URL | ||
| // Invalid URL, will fall back to default avatar | ||
| } | ||
| // Return default avatar if URL is invalid or uses disallowed protocol | ||
| // Return default avatar if URL is invalid or uses disallowed host/path/protocol | ||
| return fallbackUrl | ||
| } | ||
|
|
There was a problem hiding this comment.
I think this is a false positive, we sanitized the URL, but here, React removes the attribute values when passing in text like <img src={userInput} /> - it does not treat it as HTML content.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces a sanitizeImageUrl function to prevent XSS from malicious image URLs, which is a great security improvement. However, the implementation of the fallback logic within this function can cause performance issues and UI flickering due to the generation of a new UUID on every render for invalid URLs. I've suggested a fix to use a stable fallback URL.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
christopherholland-workday
requested review from
0xi4o,
HenryHengZJ,
igor-magun-wd and
yau-wd
2 participants
Sanitize the image URL to block potentially malicious content.
CodeQL finding 26.