Closed Bug 2029071 Opened 5 months ago Closed 5 months ago

Heap-use-after-free in [@ nsHtml5TreeOperation::SetFormElement] via foster-parent handle and custom element constructor

Categories

(Core :: DOM: HTML Parser, defect)

defect

Tracking

()

VERIFIED FIXED
151 Branch
Tracking Status
firefox-esr115 150+ fixed
firefox-esr140 150+ fixed
firefox149 --- wontfix
firefox150 + fixed
firefox151 + verified

People

(Reporter: bugmon, Assigned: edgar)

References

Details

(5 keywords, Whiteboard: [prefs-checked][bugmon:bisected,confirmed][pp1][adv-main150+r][adv-esr140.10+r][adv-esr115.35+r])

Attachments

(7 files)

Heap-use-after-free in [@ nsHtml5TreeOperation::SetFormElement] via foster-parent handle and custom element constructor

The off-main-thread HTML5 parser executes tree operations on the main thread using an array of nsIContent* handle slots. Normally each slot is populated by opCreateHTMLElement/opCreateSVGElement/opCreateMathMLElement, which also pushes a strong reference into nsHtml5DocumentBuilder::mOwnedElements, so the raw pointer in the slot cannot dangle. However, opGetFosterParent (parser/html/nsHtml5TreeOperation.cpp:1039) writes aTable->GetParent() directly into a handle slot without adding it to mOwnedElements. If the table has been re-parented by script into an arbitrary script-created element, that element is held only by JS/DOM references and can be freed while the handle slot still points at it.

When a form-associated element such as <input> is foster-parented inside <form><table>, the tree-op queue contains opGetFosterParent (capturing the table's current parent into handle h), then opCreateHTMLElement(input, intendedParent=h), then opSetFormElement(input, form, intendedParent=h). For a customized built-in (<input is="x-input">), opCreateHTMLElement calls nsHtml5AutoPauseUpdate and synchronously runs the author-defined constructor via CustomElementRegistry::Upgrade. The constructor can drop the only references to the foster parent and trigger GC/CC, freeing it. The immediately-following opSetFormElement then dereferences *h and calls aParent->SubtreeRoot() on freed memory.

The testcase uses an autonomous custom element <x-a> (also foster-parented) whose constructor moves the <table> into a fresh detached <div> D, so that when the <input is="x-input"> is processed, opGetFosterParent captures D. The x-input constructor then empties D, nulls the JS reference, and forces GC/CC (FuzzingFunctions is used only to make GC deterministic; the bug exists with natural GC). ASAN reports a read of the freed HTMLDivElement's mSubtreeRoot field. This is reachable from any web page with default settings (custom elements and customized built-ins are enabled by default).

Build Info

Affected Code

File: parser/html/nsHtml5TreeOperation.cpp, line 1039-1045

nsresult operator()(const opGetFosterParent& aOperation) {
  nsIContent* table = *(aOperation.mTable);
  nsIContent* stackParent = *(aOperation.mStackParent);
  nsIContent* fosterParent = GetFosterParent(table, stackParent);
  *aOperation.mParentHandle = fosterParent;   // raw pointer, not owned by mOwnedElements
  return NS_OK;
}

File: parser/html/nsHtml5TreeOperation.cpp, line 805-809

nsIContent* nsHtml5TreeOperation::GetFosterParent(nsIContent* aTable,
                                                  nsIContent* aStackParent) {
  nsIContent* tableParent = aTable->GetParent();
  return IsElementOrTemplateContent(tableParent) ? tableParent : aStackParent;
}

File: parser/html/nsHtml5TreeOperation.cpp, line 557-568

if (customElementDefinition) {
  // This will cause custom element constructors to run.
  AutoSetThrowOnDynamicMarkupInsertionCounter
      throwOnDynamicMarkupInsertionCounter(document);
  nsHtml5AutoPauseUpdate autoPauseContentUpdate(aBuilder);
  {
    nsAutoMicroTask mt;
  }
  AutoCEReaction autoCEReaction(
      document->GetDocGroup()->CustomElementReactionsStack(), nullptr);
  return DoCreateElement(nullptr);   // runs author script synchronously
}

File: parser/html/nsHtml5TreeOperation.cpp, line 957-960

nsresult operator()(const opSetFormElement& aOperation) {
  SetFormElement(*(aOperation.mContent), *(aOperation.mFormElement),
                 *(aOperation.mIntendedParent));   // *mIntendedParent is the dangling foster-parent handle
  return NS_OK;
}

File: parser/html/nsHtml5TreeOperation.cpp, line 705-720

void nsHtml5TreeOperation::SetFormElement(nsIContent* aNode, nsIContent* aForm,
                                          nsIContent* aParent) {
  RefPtr formElement = HTMLFormElement::FromNodeOrNull(aForm);
  ...
  if (formControl &&
      formControl->ControlType() !=
          FormControlType::FormAssociatedCustomElement &&
      !aNode->AsElement()->HasAttr(nsGkAtoms::form) &&
      aForm->SubtreeRoot() == aParent->SubtreeRoot()) {   // UAF: aParent is freed
    formControl->SetForm(formElement);
  } ...
}

File: parser/html/nsHtml5DocumentBuilder.cpp, line 14-15

NS_IMPL_CYCLE_COLLECTION_INHERITED(nsHtml5DocumentBuilder, nsContentSink,
                                   mOwnedElements)

opGetFosterParent stores an un-owned raw nsIContent* (the table's current parent, which can be an arbitrary script-created element) into a tree-op handle slot. A subsequent opCreateHTMLElement for a customized built-in element runs author script synchronously, allowing that element to be freed before opSetFormElement dereferences the same handle in aParent->SubtreeRoot(). All other handle-slot writes go through nsHtml5DocumentBuilder::HoldElement which keeps a strong reference in mOwnedElements; opGetFosterParent uniquely violates this ownership invariant.

Exploit Chain

  1. Attacker page defines an autonomous custom element x-a whose constructor reparents the parser-created <table> into a fresh detached <div> D.
  2. Attacker page defines a customized built-in x-input (extends HTMLInputElement) whose constructor empties D, drops the last JS reference to D, and provokes GC/CC (memory pressure in the wild, FuzzingFunctions in the PoC).
  3. Markup <form><table><x-a></x-a><input is="x-input"></table></form> is parsed; foster-parenting is active inside the table.
  4. When <x-a> is created, its synchronous constructor moves the <table> under D.
  5. For the <input>, the parser emits opGetFosterParent which stores D (table->GetParent()) into handle slot h; D is not added to mOwnedElements.
  6. opCreateHTMLElement(input, intendedParent=h) pauses the doc update and synchronously runs the x-input constructor, which removes D's children, drops D, and triggers GC/CC; SnowWhiteKiller frees D.
  7. opSetFormElement(input, form, intendedParent=h) runs and calls aParent->SubtreeRoot() on the freed D, reading mSubtreeRoot from freed memory.
  8. With heap grooming an attacker can reclaim the freed 136-byte allocation with controlled data, influencing the SubtreeRoot() comparison and the subsequent SetForm() call, or use other consumers of the same dangling handle (e.g. intendedParent->NodeInfoManager() in opCreateHTMLElement) for further corruption toward arbitrary code execution in the content process.

Steps to Reproduce

  1. Build Firefox with AddressSanitizer and --enable-fuzzing (for deterministic GC via FuzzingFunctions).
  2. Set pref fuzzing.enabled=true (only needed to expose FuzzingFunctions for deterministic GC; the underlying bug does not depend on it).
  3. Serve the test.html file over HTTP and load it in Firefox.
  4. Observe ASAN heap-use-after-free in nsHtml5TreeOperation::SetFormElement / nsINode::SubtreeRoot().

Security Impact

  • Severity: High
  • Attacker capability: Web-content-triggerable use-after-free of a DOM element (HTMLDivElement, 136 bytes) inside the HTML5 parser tree-op executor. The freed object is dereferenced for a pointer field read (mSubtreeRoot) and the same dangling handle is also used as intendedParent for element creation (NodeInfoManager() lookup) and form association, giving an attacker who reclaims the allocation a primitive that can plausibly be developed into renderer code execution.
  • Preconditions: None beyond visiting an attacker-controlled web page. Custom elements and customized built-in elements are enabled by default. The PoC uses FuzzingFunctions to force GC/CC for reliability; in practice the free can be induced via memory pressure.

ASAN Report

==325636==ERROR: AddressSanitizer: heap-use-after-free on address 0x7560091f70a8 at pc 0x748fdfbf64a0 bp 0x7ffda893e640 sp 0x7ffda893e638
READ of size 8 at 0x7560091f70a8 thread T0 (Isolated Web Co)
    #0 0x748fdfbf649f in nsINode::SubtreeRoot() const /firefox/dom/base/nsINode.h:1266:12
    #1 0x748fdfbf649f in nsHtml5TreeOperation::SetFormElement(nsIContent*, nsIContent*, nsIContent*) /firefox/parser/html/nsHtml5TreeOperation.cpp:715:40
    #2 0x748fdfbff5fb in nsHtml5TreeOperation::Perform(...)::TreeOperationMatcher::operator()(opSetFormElement const&) /firefox/parser/html/nsHtml5TreeOperation.cpp:958:7
    #16 0x748fdfbff5fb in nsHtml5TreeOperation::Perform(nsHtml5TreeOpExecutor*, nsIContent**, bool*, bool*) /firefox/parser/html/nsHtml5TreeOperation.cpp:1326:21
    #17 0x748fdfbfe10c in nsHtml5TreeOpExecutor::RunFlushLoop() /firefox/parser/html/nsHtml5TreeOpExecutor.cpp:731:19
    #18 0x748fdfc8997b in nsHtml5ExecutorFlusher::Run() /firefox/parser/html/nsHtml5StreamParser.cpp:157:18

0x7560091f70a8 is located 88 bytes inside of 136-byte region [0x7560091f7050,0x7560091f70d8)
freed by thread T0 (Isolated Web Co) here:
    #2 0x748fe2840776 in nsIContent::Destroy() /firefox/dom/base/FragmentOrElement.cpp:130:1
    #3 0x748fdd725f53 in SnowWhiteKiller::~SnowWhiteKiller() /firefox/xpcom/base/nsCycleCollector.cpp:2657:7
    #4 0x748fdd708bf1 in nsCycleCollector::FreeSnowWhite(bool) /firefox/xpcom/base/nsCycleCollector.cpp:2848:3
    #9 0x748fe49bc08d in mozilla::dom::FuzzingFunctions_Binding::cycleCollect(...) /firefox/obj-firefox-asan/dom/bindings/./FuzzingFunctionsBinding.cpp:157:3
    #25 0x748fe25ad74b in mozilla::dom::(anonymous namespace)::DoUpgrade(...) /firefox/dom/base/CustomElementRegistry.cpp:1463:17
    #26 0x748fe25ad74b in mozilla::dom::CustomElementRegistry::Upgrade(...) /firefox/dom/base/CustomElementRegistry.cpp:1551:3
    #27 0x748fe235dd29 in nsContentUtils::NewXULOrHTMLElement(...) /firefox/dom/base/nsContentUtils.cpp:11528:9
    #30 0x748fdfbf4d12 in nsHtml5TreeOperation::CreateHTMLElement(...) /firefox/parser/html/nsHtml5TreeOperation.cpp:567:12
    #31 0x748fdfbff2fe in nsHtml5TreeOperation::Perform(...)::TreeOperationMatcher::operator()(opCreateHTMLElement const&) /firefox/parser/html/nsHtml5TreeOperation.cpp:915:17
    #43 0x748fdfbfe10c in nsHtml5TreeOpExecutor::RunFlushLoop() /firefox/parser/html/nsHtml5TreeOpExecutor.cpp:731:19

previously allocated by thread T0 (Isolated Web Co) here:
    #3 0x748fe5f7b2ec in NS_NewHTMLDivElement(...) /firefox/dom/html/HTMLDivElement.cpp:11:1
    #10 0x748fe274c37c in mozilla::dom::Document::CreateElement(...) /firefox/dom/base/Document.cpp:9056:26
    #28 0x748fe235e060 in DoCustomElementCreate(...) /firefox/dom/base/nsContentUtils.cpp:11370:17
    #32 0x748fdfbf4d12 in nsHtml5TreeOperation::CreateHTMLElement(...) /firefox/parser/html/nsHtml5TreeOperation.cpp:567:12

SUMMARY: AddressSanitizer: heap-use-after-free /firefox/dom/base/nsINode.h:1266:12 in nsINode::SubtreeRoot() const
Attached file test.html —
Attached file crash_stack.txt —
Group: core-security → dom-core-security

Henri, could you take a look at this?
I can reproduce easily.

Assignee: nobody → hsivonen
Severity: -- → S2
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [prefs-checked]

Prefs: fuzzing.enabled only gates FuzzingFunctions, used here purely for deterministic GC/CC. The crashing code path (opGetFosterParent → opSetFormElement) is unconditional and custom elements are default-on, so this is a supported config and sec-high is correct.

Fix: Among the ops that write into nsIContent** handle slots, opGetFosterParent is the only one that doesn't ensure its result is held by mOwnedElements:

  • opCreate{HTML,SVG,MathML}Element / opShallowCloneInto → call HoldElement
  • opGetDocumentFragmentForTemplate / opGetShadowRootFromHost → return content owned via RefPtr by an element already in mOwnedElements
  • opGetFosterParent → may return aTable->GetParent(), which can be an arbitrary script-created element after a custom-element constructor re-parents the table

The patch has opGetFosterParent call HoldElement(do_AddRef(fosterParent)), restoring the invariant. This covers all downstream consumers of the handle (intendedParent->NodeInfoManager() in opCreateHTMLElement, aParent->SubtreeRoot() in opSetFormElement, etc.), not just the one that crashed. Foster-parenting is an error-recovery path so the extra ref is negligible.

Verified locally: ASAN UAF reproduces on origin/main, no crash with the patch.

This is the analysis tool's suggested fix. Feel welcome to adopt it as a starting point and evolve it as needed to meet our coding standards.

Verified bug as reproducible on mozilla-central 20260403092323-a7aeacfbb1b3.
Unable to bisect testcase (Testcase reproduces on start build!):

Start: 9333e3c91a58de0296f8104864f9262cfb3d3df9 (20250404213723)
End: a7aeacfbb1b38fa0379ca2a730771516bf06d28d (20260403092323)
BuildFlags: BuildFlags(asan=True, tsan=False, debug=False, fuzzing=True, coverage=False, valgrind=False, no_opt=False, fuzzilli=False, nyx=False, searchfox=False, afl=False)

Whiteboard: [prefs-checked] → [prefs-checked][bugmon:bisected,confirmed]
Assignee: hsivonen → echen
Whiteboard: [prefs-checked][bugmon:bisected,confirmed] → [prefs-checked][bugmon:bisected,confirmed][pp1]
Status: NEW → ASSIGNED
Blocks: 2030641
Attached file (secure) —

Comment on attachment 9568180 [details]
(secure)

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: The patch suggests the issue is related to foster parenting, but I don't think it is easy to figure out how to trigger it from the patch alone.
  • 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?: Should be applied clearly
  • How likely is this patch to cause regressions; how much testing does it need?: Fix is straightforward, it should be safe.
  • Is the patch ready to land after security approval is given?: Yes
  • Is Android affected?: Yes
Attachment #9568180 - Flags: sec-approval?
Attachment #9568180 - Flags: sec-approval? → sec-approval+
Group: dom-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 151 Branch

Verified bug as fixed on rev mozilla-central 20260410211335-be5d6cb74c0a.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Status: RESOLVED → VERIFIED
Keywords: bugmon

Please add Beta, ESR140, and ESR115 uplift requests.

Flags: needinfo?(echen)

firefox-beta Uplift Approval Request

  • User impact if declined/Reason for urgency: UAF
  • Code covered by automated testing?: no
  • Fix verified in Nightly?: yes
  • Needs manual QE testing?: no
  • Steps to reproduce for manual QE testing: None
  • Risk associated with taking this patch: low
  • Explanation of risk level: Fix is straightforward, it should be safe.
  • String changes made/needed?: None
  • Is Android affected?: yes
Attachment #9569017 - Flags: approval-mozilla-beta?
Attached file (secure) —

firefox-esr115 Uplift Approval Request

  • User impact if declined/Reason for urgency: UAF
  • Code covered by automated testing?: no
  • Fix verified in Nightly?: yes
  • Needs manual QE testing?: no
  • Steps to reproduce for manual QE testing: None
  • Risk associated with taking this patch: low
  • Explanation of risk level: Fix is straightforward, it should be safe.
  • String changes made/needed?: None
  • Is Android affected?: yes
Attachment #9569018 - Flags: approval-mozilla-esr115?
Attached file (secure) —

firefox-esr140 Uplift Approval Request

  • User impact if declined/Reason for urgency: UAF
  • Code covered by automated testing?: no
  • Fix verified in Nightly?: yes
  • Needs manual QE testing?: no
  • Steps to reproduce for manual QE testing: None
  • Risk associated with taking this patch: low
  • Explanation of risk level: Fix is straightforward, it should be safe.
  • String changes made/needed?: None
  • Is Android affected?: yes
Attachment #9569019 - Flags: approval-mozilla-esr140?
Attached file (secure) —
Flags: needinfo?(echen)
Attachment #9569017 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9569019 - Flags: approval-mozilla-esr140? → approval-mozilla-esr140+
Attachment #9569018 - Flags: approval-mozilla-esr115? → approval-mozilla-esr115+

Filed bug 2031263 as a follow-up audit.

QA Whiteboard: [sec] [qa-triage-done-c151/b150]
Whiteboard: [prefs-checked][bugmon:bisected,confirmed][pp1] → [prefs-checked][bugmon:bisected,confirmed][pp1][adv-main150+r][adv-esr140.10+r][adv-esr115.35+r]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: