Heap use-after-free in [@ Document::HandlePendingFullscreenRequests] via popover beforetoggle re-entrancy
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
People
(Reporter: bugmon, Assigned: smaug)
References
Details
(5 keywords, Whiteboard: [pp1][adv-main150+r][adv-esr140.10+r][adv-esr115.35+r])
Attachments
(4 files, 1 external file)
PendingFullscreenChangeList stores all pending FullscreenRequest objects in a process-global static LinkedList<FullscreenChange>, and PendingFullscreenChangeList::Iterator caches the current list node in a raw FullscreenChange* mCurrent. Document::HandlePendingFullscreenRequests iterates this list and, for each request, calls ApplyFullscreen(). ApplyFullscreen() invokes HideAllPopoversUntil(), which synchronously dispatches a 'beforetoggle' event to content script before the iterator advances past the next node.
From inside the beforetoggle handler, content can remove a same-process <iframe> whose document owns the next pending FullscreenRequest in the list. Removing the iframe destroys its docshell synchronously (via nsFrameLoader::DestroyDocShell at the next script-blocker release), which calls Document::OnPageHide -> ClearPendingFullscreenRequests. ClearPendingFullscreenRequests creates its own iterator over the same global list, takes ownership of the iframe's request via UniquePtr, and deletes it when the UniquePtr goes out of scope. The outer iterator's mCurrent now dangles. On the next loop turn, TakeAndNext() -> TakeAndNextInternal() calls mCurrent->removeAndGetNext() on the freed 56-byte allocation, reading mNext and then performing a linked-list unlink that writes through attacker-influenceable pointers if the slot has been reclaimed.
The bug is reachable from ordinary web content: a single user-activation gesture is sufficient to queue two fullscreen requests (one for the top document, one for a same-origin iframe), open an auto popover, and attach a beforetoggle listener that removes the iframe. The non-default prefs and FuzzingFunctions.spinEventLoopFor in the testcase only stand in for the user click and for naturally returning to the event loop so the parent's DOMFullscreen:Entered reply can be processed; they do not change the vulnerable code path.
Build Info
- Branch: main
- Revision: 6164ea4bacaeaed1f617c11911df7fc32f2e6ec2
- Timestamp: 2026-04-02T19:36:24+00:00
Affected Code
File: dom/base/Document.cpp, line 16769-16780
bool Document::HandlePendingFullscreenRequests(Document* aDoc) {
bool handled = false;
PendingFullscreenChangeList::Iterator<FullscreenRequest> iter(
aDoc, PendingFullscreenChangeList::eDocumentsWithSameRoot);
while (!iter.AtEnd()) {
UniquePtr<FullscreenRequest> request = iter.TakeAndNext();
Document* doc = request->Document();
if (doc->ApplyFullscreen(std::move(request))) { // <-- can run script that mutates sList and frees iter.mCurrent
handled = true;
}
}
return handled;
}
File: dom/base/Document.cpp, line 15345-15376
UniquePtr<T> TakeAndNextInternal() {
FullscreenChange* thisChange = mCurrent;
MOZ_ASSERT(thisChange->Type() == T::kType);
mCurrent = mCurrent->removeAndGetNext(); // <-- UAF: mCurrent points into freed 56-byte FullscreenRequest
return WrapUnique(static_cast<T*>(thisChange));
}
...
FullscreenChange* mCurrent; // raw pointer into static LinkedList<FullscreenChange> sList
RefPtr<BrowsingContext> mRootBCForIteration;
File: dom/base/Document.cpp, line 16822-16837
Element* elem = aRequest->Element();
RefPtr<nsINode> hideUntil = elem->GetTopmostPopoverAncestor(
PopoverAttributeState::Hint, nullptr, false);
if (!hideUntil) {
hideUntil = elem->GetTopmostPopoverAncestor(PopoverAttributeState::Auto,
nullptr, false);
}
if (!hideUntil) {
hideUntil = OwnerDoc();
}
RefPtr<Document> doc = aRequest->Document();
doc->HideAllPopoversUntil(*hideUntil, false, true); // <-- fires synchronous 'beforetoggle' to content
File: dom/base/Document.cpp, line 12709
ClearPendingFullscreenRequests(this); // called from OnPageHide when the iframe's docshell is destroyed
File: dom/base/Document.cpp, line 16784-16791
void Document::ClearPendingFullscreenRequests(Document* aDoc) {
PendingFullscreenChangeList::Iterator<FullscreenRequest> iter(
aDoc, PendingFullscreenChangeList::eInclusiveDescendants);
while (!iter.AtEnd()) {
UniquePtr<FullscreenRequest> request = iter.TakeAndNext();
request->MayRejectPromise("Fullscreen request aborted");
} // <-- UniquePtr dtor frees the FullscreenRequest that the outer iterator's mCurrent still points to
}
HandlePendingFullscreenRequests iterates the global pending list with a raw-pointer iterator while ApplyFullscreen() can synchronously run content script (via popover 'beforetoggle'). That script can cause OnPageHide -> ClearPendingFullscreenRequests on a subdocument, which removes-and-deletes the exact list node the outer iterator's mCurrent points to. The next TakeAndNext() then dereferences and unlinks freed memory.
Exploit Chain
- Attacker page contains an element to fullscreen, an auto popover that is shown, and a same-origin <iframe allowfullscreen>.
- On a user click (transient activation), the page calls element.requestFullscreen() on the top document and iframe.contentDocument.body.requestFullscreen(); both FullscreenRequest objects are appended to the global PendingFullscreenChangeList::sList.
- The page registers a 'beforetoggle' listener on the open popover that calls iframe.remove() when newState === 'closed'.
- The window enters fullscreen; the parent process replies with DOMFullscreen:Entered, which calls nsDOMWindowUtils::HandleFullscreenRequests -> Document::HandlePendingFullscreenRequests in the content process.
- The iterator takes the first request and advances mCurrent to the iframe's FullscreenRequest node, then calls ApplyFullscreen() for the first request.
- ApplyFullscreen() calls HideAllPopoversUntil(), which fires 'beforetoggle' synchronously; the handler removes the iframe.
- Iframe removal runs nsFrameLoader::DestroyDocShell -> Document::OnPageHide -> ClearPendingFullscreenRequests, which removes the iframe's FullscreenRequest from sList and deletes it.
- Control returns to HandlePendingFullscreenRequests; iter.AtEnd() is false (mCurrent is non-null but dangling), so iter.TakeAndNext() calls mCurrent->removeAndGetNext() on freed memory.
- removeAndGetNext() reads mNext from the freed chunk and performs a doubly-linked-list unlink (writes to *mPrev and *mNext). By spraying 56-byte allocations between the free and the use, an attacker can control mPrev/mNext to obtain an arbitrary-pointer write primitive, leading to code execution in the content process.
Steps to Reproduce
- Set prefs: full-screen-api.allow-trusted-requests-only=false (simulates user activation), full-screen-api.transition-duration.enter="0 0", full-screen-api.transition-duration.leave="0 0", full-screen-api.enabled=true. (fuzzing.enabled is set by the harness for FuzzingFunctions.spinEventLoopFor, which only stands in for returning to the event loop.)
- Load test.html in an ASAN build of Firefox.
- The page creates a same-origin iframe, shows an auto popover, registers a beforetoggle handler that removes the iframe, queues two requestFullscreen() calls, then spins the event loop so the parent's DOMFullscreen:Entered reply is processed.
- Observe AddressSanitizer: heap-use-after-free in LinkedListElement<FullscreenChange>::removeAndGetNext() under Document::HandlePendingFullscreenRequests.
Security Impact
- Severity: High
- Attacker capability: Use-after-free of a 56-byte FullscreenRequest LinkedList node in the content process. removeAndGetNext() reads a pointer from freed memory and then performs a doubly-linked-list unlink that writes through that pointer and its peer; with heap grooming this yields an arbitrary write primitive and likely renderer code execution.
- Preconditions: Web content must obtain transient user activation (a single click) to call requestFullscreen(). A same-origin (in-process) iframe and an open auto popover are required; both are fully under the attacker's control. No non-shipping configuration is required - the prefs in the testcase only simulate the user gesture and remove animation delays.
ASAN Report
==1506==ERROR: AddressSanitizer: heap-use-after-free on address 0x778574746fa0 at pc 0x77254daaf549 bp 0x7ffc84364f00 sp 0x7ffc84364ef8
READ of size 8 at 0x778574746fa0 thread T0 (Isolated Web Co)
#0 0x77254daaf548 in mozilla::LinkedListElement<mozilla::FullscreenChange>::getNext() /firefox/obj-firefox-asan/dist/include/mozilla/LinkedList.h:207:30
#1 0x77254daaf548 in mozilla::LinkedListElement<mozilla::FullscreenChange>::removeAndGetNext() /firefox/obj-firefox-asan/dist/include/mozilla/LinkedList.h:258:17
#2 0x77254daaf548 in mozilla::dom::PendingFullscreenChangeList::Iterator<mozilla::FullscreenRequest>::TakeAndNextInternal() /firefox/dom/base/Document.cpp:15348:28
#3 0x77254daaf548 in mozilla::dom::PendingFullscreenChangeList::Iterator<mozilla::FullscreenRequest>::TakeAndNext() /firefox/dom/base/Document.cpp:15328:25
#4 0x77254daaf548 in mozilla::dom::Document::HandlePendingFullscreenRequests(mozilla::dom::Document*) /firefox/dom/base/Document.cpp:16774:49
#5 0x77254d6e7a14 in nsDOMWindowUtils::HandleFullscreenRequests(bool*) /firefox/dom/base/nsDOMWindowUtils.cpp:3536:14
0x778574746fa0 is located 0 bytes inside of 56-byte region [0x778574746fa0,0x778574746fd8)
freed by thread T0 (Isolated Web Co) here:
#4 0x77254da132b5 in mozilla::dom::Document::ClearPendingFullscreenRequests(mozilla::dom::Document*) /firefox/dom/base/Document.cpp:16790:3
#5 0x77254da7d425 in mozilla::dom::Document::OnPageHide(bool, mozilla::dom::EventTarget*, bool) /firefox/dom/base/Document.cpp:12709:3
#6 0x772556085807 in nsDocumentViewer::PageHide(bool) /firefox/layout/base/nsDocumentViewer.cpp:1308:14
#8 0x7725569b62dd in nsDocShell::Destroy() /firefox/docshell/base/nsDocShell.cpp:4351:9
#9 0x77254deb82d2 in nsFrameLoader::DestroyDocShell() /firefox/dom/base/nsFrameLoader.cpp:1987:20
#23 0x77254def20b4 in nsINode::RemoveChildNode(...) /firefox/dom/base/nsINode.cpp:2527:1
#25 0x77254deede81 in nsINode::Remove() /firefox/dom/base/nsINode.cpp:2206:11
previously allocated by thread T0 (Isolated Web Co) here:
#3 0x77254db159ea in mozilla::FullscreenRequest::Create(...) /firefox/obj-firefox-asan/dist/include/mozilla/FullscreenChange.h:83:9
#4 0x77254db159ea in mozilla::dom::Element::RequestFullscreen(...) /firefox/dom/base/Element.cpp:4757:18
SUMMARY: AddressSanitizer: heap-use-after-free
| Reporter | ||
Comment 1•7 months ago
|
||
| Reporter | ||
Comment 2•7 months ago
|
||
| Reporter | ||
Comment 3•7 months ago
|
||
| Reporter | ||
Comment 4•7 months ago
|
||
Updated•7 months ago
|
Comment 6•7 months ago
|
||
Bug 2014136 also involves popover beforetoggle re-entrancy, but this one seems to involve full screen while the other does not.
| Assignee | ||
Updated•7 months ago
|
| Assignee | ||
Comment 8•7 months ago
•
|
||
oops sorry, I have the patch, well, AI had. That is very reasonable patch.
In the spec level the issue is kind of happening in https://fullscreen.spec.whatwg.org/#dom-element-requestfullscreen:~:text=Fullscreen%20element%20within%20doc%2E
Or rather, the spec does collect the relevant entries before applying fullscreen.
| Assignee | ||
Comment 9•7 months ago
|
||
| Assignee | ||
Comment 10•7 months ago
|
||
The patch seems to apply cleanly to esr115 too.
Updated•7 months ago
|
| Assignee | ||
Comment 12•7 months ago
|
||
Comment on attachment 9566212 [details]
(secure)
Security Approval Request
- How easily could an exploit be constructed based on the patch?: The code does pinpoint where the issue is
- Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
- Which branches (beta, release, and/or ESR) are affected by this flaw, and do the release status flags reflect this affected/unaffected state correctly?: all
- If not all supported branches, which bug introduced the flaw?: None
- Do you have backports for the affected branches?: Yes
- If not, how different, hard to create, and risky will they be?: The same patch seems to apply everywhere locally
- How likely is this patch to cause regressions; how much testing does it need?: Should be relatively safe. I run quite a few tests locally.
- Is the patch ready to land after security approval is given?: Yes
- Is Android affected?: Yes
ESR Uplift Approval Request
- If this is not a sec:{high,crit} bug, please state case for ESR consideration:
- User impact if declined:
- Fix Landed on Version:
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky):
| Assignee | ||
Comment 13•7 months ago
|
||
Comment on attachment 9566212 [details]
(secure)
Beta/Release Uplift Approval Request
- User impact if declined/Reason for urgency:
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: Yes
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky):
- String changes made/needed:
- Is Android affected?: Yes
Updated•7 months ago
|
Comment 14•7 months ago
|
||
Comment 15•7 months ago
|
||
Updated•7 months ago
|
Comment 16•7 months ago
|
||
I attempted to reproduce this on ESR115 with the attached testcase and prefs (plus dom.element.popover.enabled=true) on an ASAN build and did not get a crash. Looking at the source, ESR115's Document::ApplyFullscreen calls HideAllPopoversWithoutRunningScript(), which invokes HideAllPopoversUntil(*this, false, /*aFireEvents=*/false). With aFireEvents=false, HidePopover does not dispatch the beforetoggle event, so the content-script reentrancy that would remove the iframe (and free the next pending FullscreenRequest) from within the HandlePendingFullscreenRequests loop is not reachable on ESR115. I believe ESR115 is not affected by this specific UAF path and am not transplanting the patch. Please correct me if the reentrancy is reachable via a different path.
This is an automated analysis result. If this result is incorrect please add a needinfo and feel free to correct the error.
Comment 17•7 months ago
|
||
Comment on attachment 9566212 [details]
(secure)
Approved for 150.0b8
Updated•7 months ago
|
Comment 18•7 months ago
|
||
| uplift | ||
Comment 19•7 months ago
|
||
Comment on attachment 9566212 [details]
(secure)
Approved for 140.10esr.
Updated•7 months ago
|
Comment 20•7 months ago
|
||
| uplift | ||
Comment 21•7 months ago
|
||
Comment on attachment 9566212 [details]
(secure)
Approved for 115.35esr.
Updated•7 months ago
|
Comment 22•7 months ago
|
||
| uplift | ||
Updated•7 months ago
|
Updated•6 months ago
|
Updated•2 months ago
|
Description
•