Repository navigation
Reject non-image data: URLs in the Base64 → Image decoder - #147
Conversation
normalizeBase64() passed any pasted `data:...` URL straight through to an <img src>, including non-image MIME types (e.g. data:text/html). <img> never executes markup/script from its src, but CodeQL's DOM-XSS query still flags handing arbitrary attacker-controlled text to a DOM sink without validating it first. Now only data: URLs whose declared type is image/* are accepted; anything else surfaces the existing "invalid or unsupported base64" error instead of silently reaching the DOM.
|
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The input validation is a sound hardening and the error-state fix is correct, but this change does not actually clear the js/xss-through-dom alert it targets — CodeQL does not model a regex test as a sanitizer, so the tainted value still reaches <img src>. Confirm against the next CodeQL scan and, if the alert persists, dismiss it as mitigated rather than expecting this guard to close it.
Reviewed changes
- Reject non-image
data:URLs —normalizeBase64now returnsnullunless adata:input matches/^data:image\//i, sodata:text/html,...can no longer reach<img src>. Non-data:inputs keep the magic-byte detection anddata:image/png;base64,${s}fallback. - Surface the error for rejected input — the decode error block now also shows when the input is non-empty but
decodeSrc == null, replacing the previous silent broken-<img>state.
Important
Pullfrog covered this run's model usage. DeepSeek Flash is fast and cheap — expect lighter work than a frontier model. This model allowance is temporary and separate from your Pullfrog plan. Connect a model-provider subscription or API key →
DeepSeek Flash (model usage covered by Pullfrog) ｜ 𝕏
| function normalizeBase64(raw: string): string | null { | ||
| const s = raw.trim(); | ||
| if (s.startsWith('data:')) return s; | ||
| if (s.startsWith('data:')) return /^data:image\//i.test(s) ? s : null; |
There was a problem hiding this comment.
CodeQL's js/xss-through-dom does not treat this regex as a sanitizer, so the flow from the textarea to <img src> is unchanged and the alert should still fire after this change. The validation is still worth keeping as defense-in-depth, but plan to dismiss the alert in the Security tab (like the ResponsePanel iframe) rather than expecting this guard to close it.
Technical details
# `js/xss-through-dom` is not satisfied by the regex guard
## Affected sites
- `src/components/tools/ImageBase64Tool.tsx:26` — `/^data:image\//i.test(s) ? s : null` is not a modeled sanitizer; `s` (and the `data:image/png;base64,${s}` fallback at line 35) still flows to `decodeSrc`.
- `src/components/tools/ImageBase64Tool.tsx:214` — `<img src={decodeSrc}>`, matched by `ReactAttributeWriteUrlSink`.
## Why the alert persists
- `XssThroughDomConfig.isSink` = `DomBasedXss::Sink`. `DomBasedXss::WriteUrlSink` includes `ClientSideUrlRedirect::Sink` where `isXssSink()`, and `ReactAttributeWriteUrlSink` matches React JSX attributes named `src` on lowercase HTML elements (`this = attr.getValue().flow()`).
- `XssThroughDomConfig.isBarrier` = `DomBasedXss::Sanitizer` + `isOptionallySanitizedNode` + `MakeBarrierGuard<BarrierGuard>`, where `BarrierGuard` is `Xss::Shared::BarrierGuard`, whose only concrete subclass is `ContainsHtmlGuard` (a regex character class matching all of `"`, `&`, `<`, `>`). `PrefixStringSanitizer` (a `StringOps::StartsWith` guard) is only wired into the deprecated `Configuration`, and a regex `.test()` is not a `startsWith` regardless.
- Taint still reaches the sink through both the `data:image/` pass-through and the non-`data:` fallback, so `XssThroughDomFlow::flowPath` is unchanged.
## Required outcome
- Either accept that the runtime validation is the deliverable and dismiss the CodeQL alert as mitigated/false-positive, or, if the alert must clear, restructure so the value handed to `src` carries no taint from `decodeInput` (or uses a CodeQL-modeled sanitizer). Do not record the alert as resolved without confirming on the next scan.
## Suggested approach (optional)
- Re-run CodeQL on this branch before relying on the fix; the validation still meaningfully narrows what reaches the DOM regardless of whether the alert clears.
## Open questions for the human
- Is the goal to clear the CodeQL alert, or only to harden the sink? That determines whether the current change is sufficient.
2 participants
Summary
CodeQL flagged
src/components/tools/ImageBase64Tool.tsx:210as "DOM text reinterpreted as HTML" (high severity):normalizeBase64()passed any pasteddata:...URL straight through to<img src>, including non-image MIME types.<img>never executes markup/script from itssrc, so this wasn't practically exploitable, but the sink still hands attacker-controlled text to the DOM without validating its type first.data:URLs whose declared type isimage/*are accepted. Anything else (e.g.data:text/html,...) is rejected and surfaces the existing "invalid or unsupported base64" error instead of reaching the DOM at all.data:URL previously set a broken<img>src silently instead of showing the error state.Note: this is unrelated to PR #145 (Marketplace tab) —
ImageBase64Tool.tsxisn't touched by that PR's diff. The 2 CodeQL alerts reported against PR #145 (this file +ResponsePanel.tsx) are pre-existing onmain; GitHub's default-setup CodeQL attributes them to whichever PR triggers the first scan against a fresh baseline, which is why they showed up as "new alerts" there.The second alert (
ResponsePanel.tsx's sandboxed<iframe sandbox="" srcDoc={response.body}>HTML preview) is intentional, correctly-mitigated behavior —sandbox=""is the most restrictive value and fully disables script execution, same-origin access, forms, popups, and top-navigation. Recommend dismissing that specific alert in the Security tab as a false positive/mitigated rather than removing the HTML-preview feature.Test plan
npx tsc --noEmit— cleannpx vitest run— 1333/1333 passing🤖 Generated with Claude Code
https://claude.ai/code/session_01BCypCuViyQDKGWxWspXKs2
Generated by Claude Code