Skip to content

fix: snapshot attributes in DOM.removeAttributes to avoid skipping them - #8189

Merged
HarelM merged 2 commits into
maplibre:mainfrom
0xKirisame:fix/dom-sanitize-live-attributemap-bypass
Aug 18, 2026
Merged

HarelM merged 2 commits into
maplibre:mainfrom
0xKirisame:fix/dom-sanitize-live-attributemap-bypass

Conversation

@0xKirisame

Copy link
Copy Markdown
Contributor

Launch Checklist

  • Confirm your changes do not include backports from Mapbox projects (unless with compliant license)
  • Briefly describe the changes in this PR.
  • Link to related issues.
  • Write tests for all new functionality.
  • Add an entry to CHANGELOG.md under the ## main section.
  • Confirm you have read our AI policy here.

What

Fixes a bug in DOM.removeAttributes where iterating the live NamedNodeMap from elem.attributes while calling elem.removeAttribute(name) skips the attribute directly after a removed one. When two dangerous attributes are consecutive, the first is removed and the second survives into the sanitised output of DOM.sanitize.

This is exploitable in the attribution control: style-supplied attribution strings pass through DOM.sanitize before being assigned to innerHTML (src/ui/control/attribution_control.ts:177). A payload such as <details open onload="1" ontoggle="alert(1)">x</details> keeps ontoggle, and the toggle event fires on insertion, so the handler executes with no user interaction.

Why

The current unit tests in src/util/dom.test.ts only exercise a single dangerous attribute per element, which is why the skip was never caught.

How

  • Snapshot the attribute collection with Array.from(elem.attributes) before iterating, so removals no longer affect iteration order.
  • Added two regression tests covering multiple consecutive dangerous attributes, verified to fail on the old code and pass with the fix.

Out of scope

Found and verified with an autonomous logic-audit harness I built. All testing was against local builds of this library; no MapLibre-operated service was touched. Coordinated disclosure in progress with the security team, hence the separate report; happy to share details once the fix is released.

  • Assisted-By: opencode (deepseek-v4-flash-free) for the audit harness; the one-line fix and regression tests were reviewed and committed by a human.

Iterating the live NamedNodeMap from elem.attributes while calling removeAttribute skips the attribute directly after a removed one, so a second dangerous attribute can survive DOM.sanitize. Snapshot the collection with Array.from before iterating.

@HarelM HarelM left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

THANKS

@HarelM
HarelM enabled auto-merge (squash) August 18, 2026 12:58
@HarelM
HarelM merged commit 1da69f3 into maplibre:main Aug 18, 2026
23 checks passed
@codecov

codecov Bot commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.92%. Comparing base (ef90e66) to head (80160b4).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8189   +/-   ##
=======================================
  Coverage   93.92%   93.92%           
=======================================
  Files         290      290           
  Lines       24916    24916           
  Branches     6575     6575           
=======================================
  Hits        23402    23402           
  Misses       1514     1514           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MathiasWP

Copy link
Copy Markdown

Why are you not using https://github.com/cure53/dompurify?

@SalmanAljardan

Copy link
Copy Markdown

Why are you not using https://github.com/cure53/dompurify?

adding a new dependency to the project is an architectural change instead of a simple one line patch which was much easier for the reviewer to ensure its correctness

changing the default built-in sanitizer to replace with a new dependency is the choice of the project architect, as a researcher I simply shipped a patch to the exploit and added a new rule to the sanitizer

@HarelM

HarelM commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Before hacking this code I tried to find a slim dependecy, but everything was very big in terms of bundle size.

@MathiasWP

Copy link
Copy Markdown

@HarelM but doesn't the size of https://github.com/cure53/dompurify reflect that robust security is a lot of work? It's been hardened over many years, with more edge cases covered than most people know about.

@HarelM

HarelM commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

True, I don't disagree.
But maplibre is a library that you use alongside others. You can easily sanitize the attribution in the sources that you send down to maplibre or disable the attribution control altogether.
Building a full app is up to you and you can decide if you want to use larger dom sanitizers or not.

renovate Bot added a commit to cigaleapp/cigale that referenced this pull request Aug 25, 2026
##### [v6.4.1](https://github.com/maplibre/maplibre-gl-js/blob/HEAD/CHANGELOG.md#641)

##### 🐞 Bug fixes

- Fix `DOM.sanitize` leaving dangerous attributes behind when multiple consecutive attributes are present. Iterating the live `NamedNodeMap` from `elem.attributes` while calling `removeAttribute` skipped the attribute directly after a removed one, so a second dangerous attribute (for example an `ontoggle` on a `<details open>` element) could survive sanitisation and later execute ([#8189](maplibre/maplibre-gl-js#8189)) (by [@0xKirisame](https://github.com/0xKirisame))
- Give custom layers the live globe transition in `CustomRenderMethodInput.defaultProjectionData.projectionTransition`, which was hardcoded to 1 for the whole globe/mercator transition, so a custom layer jumped straight to the fully bent globe while every other layer eased ([#8169](maplibre/maplibre-gl-js#8169)) (by [@mondsichtung](https://github.com/mondsichtung))
##### [v6.4.0](https://github.com/maplibre/maplibre-gl-js/blob/HEAD/CHANGELOG.md#640)

##### ✨ Features and improvements

- Avoid a per-query `Array.sort()` in cross-tile symbol matching (`TileLayerIndex.findMatches`), claiming the lowest-index unclaimed candidate in a single pass instead; reduces main-thread symbol-placement cost on dense/coincident symbol layers ([#7797](maplibre/maplibre-gl-js#7797)) (by [@pholmstr](https://github.com/pholmstr))
- Use `texelFetch` for exact DEM and color-relief elevation stop lookups instead of normalized texture coordinate arithmetic ([#7640](maplibre/maplibre-gl-js#7640)) (by [@johncarmack1984](https://github.com/johncarmack1984))
- Make default draggable markers keyboard-focusable and movable with the arrow keys (1 px per press, 10 px with Shift); custom marker elements stay application-owned ([#8020](maplibre/maplibre-gl-js#8020)) (by [@smmariquit](https://github.com/smmariquit))

##### 🐞 Bug fixes

- Fix a permanent frame rate degradation after switching styles: every sprite reload marked its images as updated forever, making every in-view tile re-check and re-upload them on every frame. Also stop leaking the images of a replaced sprite, which were never removed from the image manager ([#8052](maplibre/maplibre-gl-js#8052)) (by [@HarelM](https://github.com/HarelM))
- Prevent a rejected missing style image resolver from blocking successfully resolved images in the same batch ([#8146](maplibre/maplibre-gl-js#8146)) (by [@birkskyum](https://github.com/birkskyum))
- Explicitly request no browser color management when decoding raster-DEM tiles so their RGB-encoded elevation values are not changed (what would otherwise happen with `gfx.color_management.mode = 1` in Firefox) ([#8125](maplibre/maplibre-gl-js#8125)) (by [@tnikkel](https://github.com/tnikkel))
- Let an abort reach an image or raster tile load that is still awaiting its `transformRequest`, so `ImageSource.updateImage` no longer loses the image it was just handed and an aborted tile is no longer fetched ([#8071](maplibre/maplibre-gl-js#8071)) (by [@mondsichtung](https://github.com/mondsichtung))
- Fix raster tiles fading in again when they are reloaded, briefly flashing the map background, most visibly when switching projection ([#8106](maplibre/maplibre-gl-js#8106)) (by [@mondsichtung](https://github.com/mondsichtung))
- Fix globe panning inverting and stalling near and across the poles by rotating the globe with a versor, keeping the drag direction consistent at every latitude. Panning also eases off as the cursor approaches the edge of the globe and continues past it, instead of stopping. The bearing is preserved while panning, as before ([#5296](maplibre/maplibre-gl-js#5296)) (by [@jcolot](https://github.com/jcolot))
- Fix `fill-extrusion-rounded-corner-distance` producing spikes: corner arcs now land on the integer tile grid, and corners created by tile clipping are left sharp ([#8153](maplibre/maplibre-gl-js#8153)) (by [@HarelM](https://github.com/HarelM))
- Fix a gesture which was held still before being released still flinging the map ([#1303](maplibre/maplibre-gl-js#1303)) (by [@zdila](https://github.com/zdila))
##### [v6.3.0](https://github.com/maplibre/maplibre-gl-js/blob/HEAD/CHANGELOG.md#630)

##### ✨ Features and improvements

- Make fired/listened map events typed. This means that `map.on("something", ...)` (and `once`, `listens`) will now give you an typescript error and better autocomplete. If you relied on firing/listening custom events via the map, this still works via the escape hatches `map.fire("something" as any)` -> `map.on("something" as any, ...)` ([#8072](maplibre/maplibre-gl-js#8072)) (by [@CommanderStorm](https://github.com/CommanderStorm))
- Let a `StyleImageInterface` give a `{renderWithWebGL}` callback as its `data`, an escape hatch for plugin developers and advanced users that renders a style image on the GPU instead of moving its pixels through the CPU. Nothing new is possible that pixels could not already express, but an image that changes often, such as an animated icon, gets more performant ([#7954](maplibre/maplibre-gl-js#7954)) (by [@lucaswoj](https://github.com/lucaswoj))
- Use integer vertex attributes for packed line data instead of float conversion ([#7640](maplibre/maplibre-gl-js#7640)) (by [@johncarmack1984](https://github.com/johncarmack1984))
- Use integer vertex attributes for packed circle, heatmap, symbol, and fill-extrusion data instead of float conversion ([#7640](maplibre/maplibre-gl-js#7640), [#8143](maplibre/maplibre-gl-js#8143)) (by [@johncarmack1984](https://github.com/johncarmack1984))
- Redesign benchmarks to use vitest bench capabilities and remove custom build for benchmarks code ([#982](maplibre/maplibre-gl-js#982)) (by [@johncarmack1984](https://github.com/johncarmack1984))

##### 🐞 Bug fixes

- Fix terrain pan/zoom gestures losing the grabbed terrain point: gestures are now solved against the elevation of the terrain under the gesture instead of the frozen center elevation, so terrain under the pointer/fingers no longer slips during moving-centroid pinches and drags ([#8067](maplibre/maplibre-gl-js#8067)) (by [@StrawberryJam22](https://github.com/StrawberryJam22))
- Fix `ImageSource`, `VideoSource` and `CanvasSource` leaking a GPU texture on every image update and on removal, and a resized texture losing its wrap and filter settings ([#8094](maplibre/maplibre-gl-js#8094)) (by [@mondsichtung](https://github.com/mondsichtung))
- Fix `map.queryRenderedFeatures()` sometimes causing "Out of bounds" error due to race condition while loading tile data ([#8064](maplibre/maplibre-gl-js#8064)) (by [@smvjohansenbouvet](https://github.com/smvjohansenbouvet))
- Fix zooming the globe with the scroll wheel or a two-finger pinch drifting away from the pointer while the globe is small on screen, instead of keeping the location under the pointer as it does when zoomed in ([#8095](maplibre/maplibre-gl-js#8095)) (by [@mondsichtung](https://github.com/mondsichtung))
- Fix projective rendering for non-parallelogram image source quads ([#7887](maplibre/maplibre-gl-js#7887)) (by [@i4innovationnet](https://github.com/i4innovationnet))
gwennlbh pushed a commit to cigaleapp/cigale that referenced this pull request Aug 25, 2026
##### [v6.4.1](https://github.com/maplibre/maplibre-gl-js/blob/HEAD/CHANGELOG.md#641)

##### 🐞 Bug fixes

- Fix `DOM.sanitize` leaving dangerous attributes behind when multiple consecutive attributes are present. Iterating the live `NamedNodeMap` from `elem.attributes` while calling `removeAttribute` skipped the attribute directly after a removed one, so a second dangerous attribute (for example an `ontoggle` on a `<details open>` element) could survive sanitisation and later execute ([#8189](maplibre/maplibre-gl-js#8189)) (by [@0xKirisame](https://github.com/0xKirisame))
- Give custom layers the live globe transition in `CustomRenderMethodInput.defaultProjectionData.projectionTransition`, which was hardcoded to 1 for the whole globe/mercator transition, so a custom layer jumped straight to the fully bent globe while every other layer eased ([#8169](maplibre/maplibre-gl-js#8169)) (by [@mondsichtung](https://github.com/mondsichtung))
##### [v6.4.0](https://github.com/maplibre/maplibre-gl-js/blob/HEAD/CHANGELOG.md#640)

##### ✨ Features and improvements

- Avoid a per-query `Array.sort()` in cross-tile symbol matching (`TileLayerIndex.findMatches`), claiming the lowest-index unclaimed candidate in a single pass instead; reduces main-thread symbol-placement cost on dense/coincident symbol layers ([#7797](maplibre/maplibre-gl-js#7797)) (by [@pholmstr](https://github.com/pholmstr))
- Use `texelFetch` for exact DEM and color-relief elevation stop lookups instead of normalized texture coordinate arithmetic ([#7640](maplibre/maplibre-gl-js#7640)) (by [@johncarmack1984](https://github.com/johncarmack1984))
- Make default draggable markers keyboard-focusable and movable with the arrow keys (1 px per press, 10 px with Shift); custom marker elements stay application-owned ([#8020](maplibre/maplibre-gl-js#8020)) (by [@smmariquit](https://github.com/smmariquit))

##### 🐞 Bug fixes

- Fix a permanent frame rate degradation after switching styles: every sprite reload marked its images as updated forever, making every in-view tile re-check and re-upload them on every frame. Also stop leaking the images of a replaced sprite, which were never removed from the image manager ([#8052](maplibre/maplibre-gl-js#8052)) (by [@HarelM](https://github.com/HarelM))
- Prevent a rejected missing style image resolver from blocking successfully resolved images in the same batch ([#8146](maplibre/maplibre-gl-js#8146)) (by [@birkskyum](https://github.com/birkskyum))
- Explicitly request no browser color management when decoding raster-DEM tiles so their RGB-encoded elevation values are not changed (what would otherwise happen with `gfx.color_management.mode = 1` in Firefox) ([#8125](maplibre/maplibre-gl-js#8125)) (by [@tnikkel](https://github.com/tnikkel))
- Let an abort reach an image or raster tile load that is still awaiting its `transformRequest`, so `ImageSource.updateImage` no longer loses the image it was just handed and an aborted tile is no longer fetched ([#8071](maplibre/maplibre-gl-js#8071)) (by [@mondsichtung](https://github.com/mondsichtung))
- Fix raster tiles fading in again when they are reloaded, briefly flashing the map background, most visibly when switching projection ([#8106](maplibre/maplibre-gl-js#8106)) (by [@mondsichtung](https://github.com/mondsichtung))
- Fix globe panning inverting and stalling near and across the poles by rotating the globe with a versor, keeping the drag direction consistent at every latitude. Panning also eases off as the cursor approaches the edge of the globe and continues past it, instead of stopping. The bearing is preserved while panning, as before ([#5296](maplibre/maplibre-gl-js#5296)) (by [@jcolot](https://github.com/jcolot))
- Fix `fill-extrusion-rounded-corner-distance` producing spikes: corner arcs now land on the integer tile grid, and corners created by tile clipping are left sharp ([#8153](maplibre/maplibre-gl-js#8153)) (by [@HarelM](https://github.com/HarelM))
- Fix a gesture which was held still before being released still flinging the map ([#1303](maplibre/maplibre-gl-js#1303)) (by [@zdila](https://github.com/zdila))
##### [v6.3.0](https://github.com/maplibre/maplibre-gl-js/blob/HEAD/CHANGELOG.md#630)

##### ✨ Features and improvements

- Make fired/listened map events typed. This means that `map.on("something", ...)` (and `once`, `listens`) will now give you an typescript error and better autocomplete. If you relied on firing/listening custom events via the map, this still works via the escape hatches `map.fire("something" as any)` -> `map.on("something" as any, ...)` ([#8072](maplibre/maplibre-gl-js#8072)) (by [@CommanderStorm](https://github.com/CommanderStorm))
- Let a `StyleImageInterface` give a `{renderWithWebGL}` callback as its `data`, an escape hatch for plugin developers and advanced users that renders a style image on the GPU instead of moving its pixels through the CPU. Nothing new is possible that pixels could not already express, but an image that changes often, such as an animated icon, gets more performant ([#7954](maplibre/maplibre-gl-js#7954)) (by [@lucaswoj](https://github.com/lucaswoj))
- Use integer vertex attributes for packed line data instead of float conversion ([#7640](maplibre/maplibre-gl-js#7640)) (by [@johncarmack1984](https://github.com/johncarmack1984))
- Use integer vertex attributes for packed circle, heatmap, symbol, and fill-extrusion data instead of float conversion ([#7640](maplibre/maplibre-gl-js#7640), [#8143](maplibre/maplibre-gl-js#8143)) (by [@johncarmack1984](https://github.com/johncarmack1984))
- Redesign benchmarks to use vitest bench capabilities and remove custom build for benchmarks code ([#982](maplibre/maplibre-gl-js#982)) (by [@johncarmack1984](https://github.com/johncarmack1984))

##### 🐞 Bug fixes

- Fix terrain pan/zoom gestures losing the grabbed terrain point: gestures are now solved against the elevation of the terrain under the gesture instead of the frozen center elevation, so terrain under the pointer/fingers no longer slips during moving-centroid pinches and drags ([#8067](maplibre/maplibre-gl-js#8067)) (by [@StrawberryJam22](https://github.com/StrawberryJam22))
- Fix `ImageSource`, `VideoSource` and `CanvasSource` leaking a GPU texture on every image update and on removal, and a resized texture losing its wrap and filter settings ([#8094](maplibre/maplibre-gl-js#8094)) (by [@mondsichtung](https://github.com/mondsichtung))
- Fix `map.queryRenderedFeatures()` sometimes causing "Out of bounds" error due to race condition while loading tile data ([#8064](maplibre/maplibre-gl-js#8064)) (by [@smvjohansenbouvet](https://github.com/smvjohansenbouvet))
- Fix zooming the globe with the scroll wheel or a two-finger pinch drifting away from the pointer while the globe is small on screen, instead of keeping the location under the pointer as it does when zoomed in ([#8095](maplibre/maplibre-gl-js#8095)) (by [@mondsichtung](https://github.com/mondsichtung))
- Fix projective rendering for non-parallelogram image source quads ([#7887](maplibre/maplibre-gl-js#7887)) (by [@i4innovationnet](https://github.com/i4innovationnet))

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@CommanderStorm

CommanderStorm commented Aug 26, 2026 •

Copy link
Copy Markdown
Contributor

@0xKirisame @HarelM should we raise an CVE about this?

@HarelM

HarelM commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

I believe I did, didn't I?

@CommanderStorm

Copy link
Copy Markdown
Contributor

advisory != CVE -> GHSA-jrc7-96c5-q579

I requested it

@HarelM

HarelM commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Ok. thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants