Closed Bug 926361 (CVE-2013-5618) Opened 11 years ago Closed 11 years ago

ASAN use-after-free in nsNodeUtils::LastRelease on anonymous node from ShowInlineTableEditingUI

Categories

(Core :: DOM: Editor, defect)

Product:
Core
Component:
DOM: Editor
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla27
Tracking Flags:
Tracking Status
firefox24 --- wontfix
firefox25 --- wontfix
firefox26 + fixed
firefox27 + fixed
firefox-esr17 --- wontfix
firefox-esr24 26+ fixed
b2g18 --- fixed
b2g-v1.1hd --- fixed
b2g-v1.2 --- fixed

People

(Reporter: nils, Assigned: smaug)

Details

(Keywords: csectype-uaf, sec-critical, Whiteboard: [asan][adv-main26+][adv-esr24.2+])

Attachments

(3 files)

crash.html
703 bytes, text/html
Details
stack.txt (ASAN output)
14.94 KB, text/plain
Details
patch
741 bytes, patch
jst
: review+
abillings
: approval-mozilla-aurora+
abillings
: approval-mozilla-beta-
abillings
: approval-mozilla-esr24+
bajaj
: approval-mozilla-b2g18+
abillings
: sec-approval+
Details | Diff | Splinter Review
Attached file crash.html —
The attached testcase crashes the latest nightly ASAN build. It requires domFuzzLite https://www.squarefree.com/extensions/domFuzzLite3.xpi
attaching ASAN output
Assignee: nobody → bugs
Keywords: csec-uaf, sec-critical
Whiteboard: [asan]
Attached patch patch — — Splinter Review
This is lovely. I'd say a bug in NS_RELEASE, but one could say a feature. So we end up last-releasing <body>, but while doing the last release, an <a> has still mParent pointing to <body> (because releasing that mParent caused the initial last-release) and we end up trying to re-unbind.
Attachment #816762 - Flags: review?(jst)
Attachment #816762 - Flags: review?(jst) → review+
Comment on attachment 816762 [details] [diff] [review] patch [Security approval request comment] How easily could an exploit be constructed based on the patch? Not easily. I haven't see this kind of crash ever before, so one needs to figure out a way to use editor in a particular way and run gc/cc just at the right moment. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? Well, the patch is rather obvious, but it is hard to combine that to the functionality of editor/ code Which older supported branches are affected by this flaw? all Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? there was a file rename nsGenericElement.cpp -> Element.cpp, but otherwise the code looks the same How likely is this patch to cause regressions; how much testing does it need? Hmm, not totally risk-free. Some code might rely on the mParent to point to somewhere while doing the release...
Attachment #816762 - Flags: sec-approval?
Attachment #816762 - Flags: approval-mozilla-esr24?
Attachment #816762 - Flags: approval-mozilla-esr17?
Attachment #816762 - Flags: approval-mozilla-beta?
Attachment #816762 - Flags: approval-mozilla-b2g18?
Attachment #816762 - Flags: approval-mozilla-aurora?
Release Management, how do you feel about taking this at this point in the cycle? I assume it is probably ok but I wanted to check.
Flags: needinfo?(release-mgmt)
(In reply to Olli Pettay [:smaug] from comment #3) > How likely is this patch to cause regressions; how much testing does it need? > Hmm, not totally risk-free. Some code might rely on the mParent to point to > somewhere while doing the release... This makes me nervous. I'd need to know more about worst case scenarios. If we do decide to take this, it'll need to land for Thursday's Beta.
Well, some code might work differently if element doesn't have parentNode anyway. Not likely, but in theory possible. And I'm talking about Gecko internal code. Crashes or anything like that should be very unlikely.
I think the possibility of a web regressions (if I'm reading your comment correctly) should preclude this from landing in our final beta.
Comment on attachment 816762 [details] [diff] [review] patch sec-approval+ for trunk. Aurora approval. Let's get it in. I want this in ESR24 but I expect we'll need to land it for the 26+ tracking release, not the current 25+ tracking. So I'm not sure how you want to track it to land it on ESR24 after we branch in two weeks. I'm going to won't fix this for ESR17 since it is done after the next release.
Attachment #816762 - Flags: sec-approval?
Attachment #816762 - Flags: sec-approval+
Attachment #816762 - Flags: approval-mozilla-esr24?
Attachment #816762 - Flags: approval-mozilla-esr24+
Attachment #816762 - Flags: approval-mozilla-esr17?
Attachment #816762 - Flags: approval-mozilla-beta?
Attachment #816762 - Flags: approval-mozilla-beta-
Attachment #816762 - Flags: approval-mozilla-aurora?
Attachment #816762 - Flags: approval-mozilla-aurora+
Tracking with the ESR24 tracking flag. Duh.
tracking-firefox-esr24: ? → 26+
Flags: needinfo?(release-mgmt)
fixed in mozilla-central https://hg.mozilla.org/mozilla-central/rev/77a86dcd14d8
Status: NEW → RESOLVED
Closed: 11 years ago
status-firefox27: affected → fixed
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Whiteboard: [asan] → [asan][land on esr24 and b2g18 when gecko26 is on beta]
Comment on attachment 816762 [details] [diff] [review] patch Just a+ on the b2g18 approval for sanity. Although this is already landed
Attachment #816762 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
I'm not able to reproduce the crash on a pre-patch ASan build. Also, I see the same debug output both before and after the patch. Example output: WARNING: Someone passed native anonymous content directly into frame construction. Stop doing that!: file /builds/slave/try-osx64-d-000000000000000000/build/layout/base/nsCSSFrameConstructor.cpp, line 6047 Nils, can you verify on at least one affected branch that the issue no longer reproduces for you? Thanks.
Whiteboard: [asan] → [asan][adv-main26+][adv-esr24.2+]
Alias: CVE-2013-5618
Group: core-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: