Repository navigation
Escape third-party data in map tooltips/popups; lazy popups - #9
Merged
Merged
Conversation
Leaflet renders string tooltip/popup content via innerHTML, and many map layers interpolated OSM, Google Places, EPA, FEMA, and HIFLD fields unescaped. With the CSP allowing 'unsafe-inline' scripts, a crafted name (e.g. <img onerror>) would execute in the app origin. - Route every external value through the shared escapeHtml and drop the two duplicate local escapers in MapPage. - Add safeHttpUrl so popup links only accept http(s) URLs (EPA superfund and TRI facility links were previously unvalidated). - Escape camera manufacturer/node id and transit stop names. - Add regression tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Pass a content function to bindPopup so popup HTML for viewport layers is only generated when a user opens a popup, instead of for every marker. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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
1 participant
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.
Security fix: stored XSS in map tooltips and popups
Leaflet sets string tooltip and popup content with
innerHTML. Many layers inserted third-party fields without escaping. Affected sources: OSM (airport, transit stop, crowd magnet and railroad names, camera manufacturer), Google Places (Costco and EMS names and addresses), EPA (Superfund and TRI facility names, addresses, URLs), FEMA (flood and tornado labels) and HIFLD (power line owner and voltage class).The CSP allows
script-src 'unsafe-inline'. A crafted value such as an OSM name of<img src=x onerror=…>would therefore run in the app's origin.Changes
escapeHtmlinsrc/utils/html.ts. The two duplicate local escapers inMapPage.tsxare removed.safeHttpUrl(): popup links (the EPA Superfund URL and the TRI facility report) accept onlyhttp:/https:URLs and are attribute-escaped. Before this, ajavascript:URL would have been rendered as-is.cameraPopupescapes the manufacturer label and URL-encodes the node id.transitPopupescapes the stop name.src/utils/html.test.tsandsrc/map/popupEscaping.test.ts.Perf: lazy popups (separate commit)
bindPopup(() => html)builds popup HTML only when a user opens a popup, not for every marker added while panning. Every captured value is a per-iterationconst.Verification
tsc -b,npm run lint,npm test(218 tests) andnpm run build(bundle budget) all pass.Follow-up suggestion
Tighten the CSP by removing
'unsafe-inline'fromscript-src. The GA inline snippet would need a hash or nonce.