← WebKit — 2026-W38
60fe8ff35a4ad47929fdd2ac017bcaea7ebc653e WebCore::Page::forEachPage should iterate a snapshot of allPages()
severity medium
class UAF
confidence 0.62
signal score 7.77
Source/WebCore/page
patch-now
Primitive: iterator invalidation / page destruction while iterating the global allPages() set
Triage note: Page::forEachPage, clearPreviousItemFromAllPages, updateStyleForAllPagesAfterGlobalChangeInEnvironment and updateControlTintsForAllPages now iterate copyToVectorOf<Ref<Page>>(allPages()) instead of allPages() directly, so reentrant callbacks that add/remove pages can no longer invalidate the iterator or free a page mid-iteration.
Triage note: Page::forEachPage, clearPreviousItemFromAllPages, updateStyleForAllPagesAfterGlobalChangeInEnvironment and updateControlTintsForAllPages now iterate copyToVectorOf<Ref<Page>>(allPages()) instead of allPages() directly, so reentrant callbacks that add/remove pages can no longer invalidate the iterator or free a page mid-iteration.
Structural signals: smartptr_member_added (+6.47), surgical (+1.3)
Commit message
WebCore::Page::forEachPage should iterate a snapshot of allPages()
<https://bugs.webkit.org/show_bug.cgi?id=320100>
<rdar://179997790>
Reviewed by Chris Dumez.
Iterate a snapshot of the global page registry so a callback that
mutates the registry cannot invalidate the iterator. `forEachPage`
walked the live `allPages()` hash set while invoking an arbitrary
caller callback per page. A callback that synchronously creates or
destroys a `Page` mutates `allPages()` (through the constructor's
`add()` and the destructor's `remove()`), which can rehash the set and
free the bucket array the iterator is still walking, leaving the loop
dereferencing freed memory.
Build a `Vector<Ref<Page>>` snapshot first, matching the existing
pattern in `LocalDOMWindow::forEachWindowInterestedInStorageEvents`.
Holding a `Ref` to each page for the loop's duration also extends the
guarantee the previous per-element `Ref` temporary already gave the
current page to every page in the snapshot.
Also snapshot `allPages()` in the other methods that iterate it live
and could re-enter through their per-page work:
`clearPreviousItemFromAllPages`,
`updateStyleForAllPagesAfterGlobalChangeInEnvironment`, and
`updateControlTintsForAllPages`. `networkStateChanged` and
`refreshPlugins` already collect into a local container before acting,
so they are left unchanged.
* Source/WebCore/page/Page.cpp:
(WebCore::Page::forEachPage):
(WebCore::Page::clearPreviousItemFromAllPages):
(WebCore::Page::updateStyleForAllPagesAfterGlobalChangeInEnvironment):
(WebCore::Page::updateControlTintsForAllPages):
Originally-landed-as: 316606.223@safari-7625-branch (674ff0d01ca3). rdar://187506399
Canonical link: https://commits.webkit.org/321423@main
Before / after
Loading diff…
On This Page