Closed Bug 1739923 (CVE-2022-22742) Opened 1 year ago Closed 1 year ago

SEGV on unknown address 0x14141414 in nsTextFragment::CharAt

Categories

(Core :: DOM: Editor, task)

Desktop
All
task

Tracking

()

VERIFIED FIXED
96 Branch
Tracking Status
firefox-esr91 96+ verified
firefox94 --- wontfix
firefox95 + wontfix
firefox96 + verified
firefox97 --- verified

People

(Reporter: sourc7, Assigned: masayuki)

References

(Depends on 1 open bug)

Details

(Keywords: csectype-bounds, sec-high, Whiteboard: [reporter-external] [client-bounty-form][Landed in 1740847][sec-survey][post-critsmash-triage][adv-main96+][adv-ESR91.5+])

Attachments

(8 files)

Attached file testcase.html

From the testcase after the insertParagraph executed, Firefox will crash with AV on 0x14141414 (32-bit) or 0x000014141414 (64-bit), the crash address is controllable through insertAdjacentText new line or space character length, on the testcase I'm using decimal 336860180 which converted to hex 0x14141414.

When execute insertAdjacentText to fill DOM with a lot of space character, the memory will increase, currently one new line or space character is equivalent to one SEGV address increase, it will be more interesting if there are workaround to optimize the memory to hit higher SEGV address.

Flags: sec-bounty?
Summary: SEGV on unknown address 0x1414141 in nsTextFragment:CharAt → SEGV on unknown address 0x14141414 in nsTextFragment:CharAt
Summary: SEGV on unknown address 0x14141414 in nsTextFragment:CharAt → SEGV on unknown address 0x14141414 in nsTextFragment::CharAt

The test case seems to involve some interaction of SVG and Editor, so I'll move it to editor for now.

I'm not really sure what is going on here, but getting the browser to crash on an address that is at least somewhat controllable sounds bad so I'll tentatively mark it sec-high.

Group: firefox-core-security → dom-core-security
Component: Security → DOM: Editor
Product: Firefox → Core

mozregression got me down to https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c68fe15a81fc2dc9fc5765f3be2573519c09b6c1&tochange=567a8768593eb06a86deb263f94d9de2d3d3e8fa

Bug 1642594 looks possibly relevant.

EDIT: Hrm, only on Windows apparently. In my Linux VM, this crashes basically since ever.

Flags: needinfo?(masayuki)

In debug build, I got hitting asserts:

###!!! ASSERTION: The offset is out of bounds: '!mParent || mOffset.value() <= mParent->Length()', file m:/fx64-dbg/dist/include\mozilla/EditorDOMPoint.h:506
#01: NS_DebugBreak (m:\src\xpcom\base\nsDebugImpl.cpp:429)
#02: mozilla::AutoEditorDOMPointChildInvalidator::InvalidateChild (m:\fx64-dbg\dist\include\mozilla\EditorDOMPoint.h:1308)
#03: mozilla::WhiteSpaceVisibilityKeeper::MakeSureToKeepVisibleWhiteSpacesVisibleAfterSplit (m:\src\editor\libeditor\WSRunObject.cpp:2381)
#04: mozilla::WhiteSpaceVisibilityKeeper::PrepareToSplitBlockElement (m:\src\editor\libeditor\WSRunObject.cpp:103)
#05: mozilla::HTMLEditor::HandleInsertParagraphInListItemElement (m:\src\editor\libeditor\HTMLEditSubActionHandler.cpp:7435)

https://searchfox.org/mozilla-central/rev/6c8d325e61b0b445ed2e04899da38c3a4c266cba/editor/libeditor/EditorDOMPoint.h#505-506
https://searchfox.org/mozilla-central/rev/6c8d325e61b0b445ed2e04899da38c3a4c266cba/editor/libeditor/WSRunObject.cpp#2353,2375

Assertion failure: aPoint.IsSetAndValid(), at m:/src/editor/libeditor/WSRunObject.cpp:2497
#01: mozilla::WSRunScanner::TextFragmentData::GetPreviousEditableCharPoint<nsCOMPtr<nsINode>,nsCOMPtr<nsIContent> > (m:\src\editor\libeditor\WSRunObject.cpp:2497)
#02: mozilla::WhiteSpaceVisibilityKeeper::MakeSureToKeepVisibleWhiteSpacesVisibleAfterSplit (m:\src\editor\libeditor\WSRunObject.cpp:2383)
#03: mozilla::WhiteSpaceVisibilityKeeper::PrepareToSplitBlockElement (m:\src\editor\libeditor\WSRunObject.cpp:103)
#04: mozilla::HTMLEditor::HandleInsertParagraphInListItemElement (m:\src\editor\libeditor\HTMLEditSubActionHandler.cpp:7435)

https://searchfox.org/mozilla-central/rev/6c8d325e61b0b445ed2e04899da38c3a4c266cba/editor/libeditor/WSRunObject.cpp#2495,2497
https://searchfox.org/mozilla-central/rev/6c8d325e61b0b445ed2e04899da38c3a4c266cba/editor/libeditor/WSRunObject.cpp#2383-2384

It seems that the former is the root cause.

Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Flags: needinfo?(masayuki)

There are 2 methods which do similar things so that we need to fix them too, but each method change should be landed separately into autoland for mozregressions because change around there may cause web-compat issues.

And also invisible white-space range and replacing white-space position should
also be tracked too.

Depends on D130722

Similar to the previous patch, this patch also make it track invisible
white-space ranges and clear outdated things.

Depends on D130723

Attachment #9249922 - Attachment description: Bug 1739923 - part 1: Trak split point at replacing collapsible white-spaces r=m_kato! → Bug 1739923 - part 1: Track split point at replacing collapsible white-spaces r=m_kato!

Comment on attachment 9249922 [details]
Bug 1739923 - part 1: Track split point at replacing collapsible white-spaces r=m_kato!

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: If the attacker knows how built-in editor handles white-space sequence, they may realize what's the problem.
  • 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 older supported branches are affected by this flaw?: 91ESR
  • If not all supported branches, which bug introduced the flaw?: Bug 1642594
  • Do you have backports for the affected branches?: Yes
  • If not, how different, hard to create, and risky will they be?: part 1 and part 2 are grafted cleanly from mozilla-central. Only part 3 requires the new patch due to the change of bug 1727844 (93) and bug 1724650 (94).
  • How likely is this patch to cause regressions; how much testing does it need?: Unfortunately, this could change the behavior of HTMLEditor when inserting text around white-spaces, deleting around white-spaces etc. I cannot say there are enough tests (WPT nor mochitests) for the text editing because there is crazy number of cases. So I recommend to land this as soon as possible for getting feedback from testers.
Attachment #9249922 - Attachment description: Bug 1739923 - part 1: Track split point at replacing collapsible white-spaces r=m_kato! → Bug 1739923 - part 1: Trak split point at replacing collapsible white-spaces r=m_kato!
Attachment #9249922 - Flags: sec-approval?
Attachment #9249923 - Flags: sec-approval?
Attachment #9249924 - Flags: sec-approval?
Attachment #9250125 - Flags: sec-approval?

These were landed in Nightly in Bug 1740847 and will ride the trains.

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Whiteboard: [reporter-external] [client-bounty-form] [verif?] → [reporter-external] [client-bounty-form] [verif?][Landed in 1740847]
Group: dom-core-security → core-security-release
Depends on: 1740847
Target Milestone: --- → 96 Branch
Attachment #9250125 - Flags: sec-approval?
Attachment #9249922 - Flags: sec-approval?
Attachment #9249923 - Flags: sec-approval?
Attachment #9249924 - Flags: sec-approval?

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Flags: needinfo?(masayuki)
Whiteboard: [reporter-external] [client-bounty-form] [verif?][Landed in 1740847] → [reporter-external] [client-bounty-form] [verif?][Landed in 1740847][sec-survey]

Done

Flags: needinfo?(masayuki)
Attachment #9249922 - Attachment description: Bug 1739923 - part 1: Trak split point at replacing collapsible white-spaces r=m_kato! → Bug 1739923 - part 1: Track split point at replacing collapsible white-spaces r=m_kato!
Flags: sec-bounty? → sec-bounty+
Flags: qe-verify+
Whiteboard: [reporter-external] [client-bounty-form] [verif?][Landed in 1740847][sec-survey] → [reporter-external] [client-bounty-form] [verif?][Landed in 1740847][sec-survey][post-critsmash-triage]

Using the test case attached, the browser crashes in Release v95.0.2, and does not crash in Beta v96.0b7 or Nightly v97.0a1.
This was tested in Windows 10, Ubuntu 20.04.3 LTS and Mac OS 11.6.2.
Closing as verified fixed.

Status: RESOLVED → VERIFIED
OS: Unspecified → All
Hardware: Unspecified → Desktop
Flags: qe-verify+
Whiteboard: [reporter-external] [client-bounty-form] [verif?][Landed in 1740847][sec-survey][post-critsmash-triage] → [reporter-external] [client-bounty-form][Landed in 1740847][sec-survey][post-critsmash-triage][adv-main96+][adv-ESR91.5+]
Attached file advisory.txt

The fix for ESR v91.5.0esr was verified in Windows 10, Ubuntu 20.04.3 LTS and Mac OS 11.6.2. The crash no longer occurs when using the test case in comment 0.
Thanks!

Alias: CVE-2022-22742
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.