fix(search): restore working fetch and escape query param against XSS#1091
Open
RAJVEER42 wants to merge 1 commit into
Open
fix(search): restore working fetch and escape query param against XSS#1091RAJVEER42 wants to merge 1 commit into
RAJVEER42 wants to merge 1 commit into
Conversation
The fetch call added in 4b9c4e3 used mode:'no-cors', which returns an opaque response whose body is unreadable. Every call to res.json() threw, routing all queries to the error handler — search has been silently broken since June 2021. Reverts to the original plain fetch(url) call. Separately, the raw URL query string was interpolated directly into searchResultsHtml before being set via innerHTML. A crafted URL like ?q=<img src=x onerror=...> would execute arbitrary JS in the visitor's browser once search was functional. Adds escapeHtml() and applies it to query before any HTML injection. These two fixes ship together: the XSS is latent while the fetch is broken, but becomes live the moment the fetch is restored.
|
Thank you for your contribution. Since you, the author, are not a member of the Ceph GitHub Org yet, our CI will not automatically run. Any member of the Ceph Org may comment "ok - to - test" (without the dashes) to allow the Jenkins jobs to run. |
|
Site built/updated successfully! https://fix/search-cors-and-xss.ceph.io |
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
3 participants
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.
What this fixes
Two bugs in
src/js/search-output.jsthat together have left blog search broken since June 2021.Bug 1 —
mode: 'no-cors'makes search permanently non-functionalCommit 4b9c4e3 added
mode: 'no-cors'to the fetch calls while debugging a CORS issue. The Fetch spec definesno-corsas returning an opaque response — the body is inaccessible and calling.json()on it always throws. Thecatchblock intercepts this and shows the "couldn't complete your search" error on every single query.The search-index and search-output files are same-origin static assets. They need no special fetch mode. This reverts to the original plain
fetch(url)call from before that commit.Bug 2 — Reflected XSS via unescaped
qURL parameterThe raw
queryvalue fromurlParams.get('q')was interpolated directly intosearchResultsHtmland then written viainnerHTML. A crafted URL such as:would execute arbitrary JavaScript in the visitor's browser.
This was latent while Bug 1 was present (fetch always failed,
renderResultswas never reached). Fixing Bug 1 without this patch would immediately expose the XSS — so both are shipped together.The fix adds a small
escapeHtmlhelper and applies it toquerybefore any HTML interpolation.What changed
method: 'GET',credentials: 'include',mode: 'no-cors'from the fetch optionsescapeHtml()function scoped insideinit()escapeHtml(query)→safeQueryin both the no-results and results-found branchesWhat was considered and rejected
textContentfor just the query display node: would require splitting the template into separate DOM operations, larger diff than needed