Crash c0000005_xul.dll!js::CallJSGetterOp

VERIFIED FIXED in Firefox 41, Firefox OS master

Status

()

Core
JavaScript Engine
VERIFIED FIXED
3 years ago
2 years ago

People

(Reporter: Tomcat, Assigned: jorendorff)

Tracking

(Blocks: 1 bug, {crash, regression, sec-critical})

unspecified
mozilla43
crash, regression, sec-critical
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +
qe-verify +

Firefox Tracking Flags

(firefox40 unaffected, firefox41+ verified, firefox42+ verified, firefox43 verified, firefox-esr38 unaffected, b2g-v2.0 unaffected, b2g-v2.0M unaffected, b2g-v2.1 unaffected, b2g-v2.1S unaffected, b2g-v2.2 unaffected, b2g-v2.2r unaffected, b2g-master fixed)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
Created attachment 8641665 [details]
windgb information from this url

found via bughunter and reproduced with a windows 7 debug build based on m-c tip

Steps to reproduce:
-> Load http://shazam.pooltracktrends.com/?id=1525
-->> Crash signature CallGetter GetExistingProperty<int> NativeGetPropertyInline<int> js::NativeGetProperty js::GetProperty

affects nightly and aurora

!exploitable 1.6.0.0
Exploitability Classification: EXPLOITABLE
Recommended Bug Title: Exploitable - Data Execution Prevention Violation starting at Unknown Symbol @ 0x0000000004497138 called from xul!js::CallJSGetterOp+0x0000000000000056 (Hash=0xff17ac06.0x050ceebd)

User mode DEP access violations are exploitable.

Updated

3 years ago
Flags: needinfo?(jdemooij)
[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

Bisected it to:

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c0ab4a5c0acf&tochange=e6f863562d50

=> bug 1125624

We're trying to invoke an old-style getter, but we crash because this pointer is a JSObject* getter.

It reproduces with the JITs disabled FWIW.
Blocks: 1125624
status-firefox40: --- → unaffected
status-firefox41: --- → affected
status-firefox42: --- → affected
tracking-firefox41: --- → ?
tracking-firefox42: --- → ?
Flags: needinfo?(jdemooij) → needinfo?(jorendorff)

Comment 2

3 years ago
Tracking for 41, 42 because regression.
tracking-firefox41: ? → +
tracking-firefox42: ? → +
Keywords: regression
Keywords: sec-critical
(Assignee)

Comment 3

3 years ago
Created attachment 8643362 [details] [diff] [review]
Fix crash after GetOwnPropertyDescriptor failed to populate all fields of desc
Attachment #8643362 - Flags: review?(jdemooij)
(Assignee)

Updated

3 years ago
Assignee: nobody → jorendorff
Status: NEW → ASSIGNED
(Assignee)

Comment 4

3 years ago
Thanks for looking into this, Jan. Definitely needs to land on the branch.
Flags: needinfo?(jorendorff)

Updated

3 years ago
Attachment #8643362 - Flags: review?(jdemooij) → review+
(Assignee)

Comment 5

3 years ago
Comment on attachment 8643362 [details] [diff] [review]
Fix crash after GetOwnPropertyDescriptor failed to populate all fields of desc

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

    It'd be challenging, maybe impossible, but there's a very clear
    goal: try to make a JS object whose in-memory representation is
    also executable code.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

    To me, yeah.

Which older supported branches are affected by this flaw?

    FF41 only

If not all supported branches, which bug introduced the flaw?

    bug 1125624

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

    No, but easy and minimal risk.

How likely is this patch to cause regressions; how much testing does it need?

    Very unlikely. Existing tests will suffice.
Attachment #8643362 - Flags: sec-approval?
Comment on attachment 8643362 [details] [diff] [review]
Fix crash after GetOwnPropertyDescriptor failed to populate all fields of desc

Sec-approval+. Please nominate for Aurora too so we can fix it there before it becomes Beta on Monday, if possible.
Attachment #8643362 - Flags: sec-approval? → sec-approval+
(Assignee)

Comment 7

3 years ago
Comment on attachment 8643362 [details] [diff] [review]
Fix crash after GetOwnPropertyDescriptor failed to populate all fields of desc

Approval Request Comment
[Feature/regressing bug #]: bug 1125624
[User impact if declined]: possibly exploitable bug
[Describe test coverage new/current, TreeHerder]: unit tests are included
[Risks and why]: minimal
[String/UUID change made/needed]: none
Attachment #8643362 - Flags: approval-mozilla-beta?
Attachment #8643362 - Flags: approval-mozilla-aurora?

Comment 8

3 years ago
Comment on attachment 8643362 [details] [diff] [review]
Fix crash after GetOwnPropertyDescriptor failed to populate all fields of desc

Approved for uplift to Aurora and Beta as it is a security fix. The patch is also simple with tests included.
Attachment #8643362 - Flags: approval-mozilla-beta?
Attachment #8643362 - Flags: approval-mozilla-beta+
Attachment #8643362 - Flags: approval-mozilla-aurora?
Attachment #8643362 - Flags: approval-mozilla-aurora+
status-b2g-v2.0: --- → unaffected
status-b2g-v2.0M: --- → unaffected
status-b2g-v2.1: --- → unaffected
status-b2g-v2.1S: --- → unaffected
status-b2g-v2.2: --- → unaffected
status-b2g-v2.2r: --- → unaffected
status-b2g-master: --- → affected
status-firefox43: --- → affected
status-firefox-esr38: --- → unaffected
(Assignee)

Comment 9

3 years ago
url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/03b1eb0b1f9bcb470c1996dedc45992eb4acef59
changeset:  03b1eb0b1f9bcb470c1996dedc45992eb4acef59
user:       Jason Orendorff <jorendorff@mozilla.com>
date:       Tue Aug 04 18:16:08 2015 -0500
description:
Bug 1189744 - Fix crash after GetOwnPropertyDescriptor failed to populate all fields of desc. r=jandem, a=abillings.
https://hg.mozilla.org/mozilla-central/rev/03b1eb0b1f9b
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-b2g-master: affected → fixed
status-firefox41: affected → fixed
status-firefox42: affected → fixed
status-firefox43: affected → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
This was found by fuzzers in bug 1183448 >2 weeks before this one.

If the 2 bugs are dupes, it would be nice to prevent this from happening again by working on such s-s fuzzer bugs asap.
Duplicate of this bug: 1183448

Updated

3 years ago
Group: core-security → core-security-release
Flags: qe-verify+
QA Contact: kjozwiak
Reproduced the crash using the following builds:
- http://inbound-archive.pub.build.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-asan/1438369481/
- https://archive.mozilla.org/pub/firefox/nightly/2015/07/2015-07-31-mozilla-central-debug/

Went through verification using the following builds:
- https://archive.mozilla.org/pub/firefox/nightly/2015-09-11-03-02-04-mozilla-central/
- http://inbound-archive.pub.build.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-asan/1441965724/
- https://archive.mozilla.org/pub/firefox/nightly/2015-09-11-00-41-12-mozilla-aurora/
- https://archive.mozilla.org/pub/firefox/candidates/41.0b9-candidates/build1/

OS's Used:

- OSX 10.10.5 x64 -> PASSED
- Win 10 x64 (VM) -> PASSED
- Ubuntu 14.04.3 (VM) x64 -> PASSED

Test Cases:

- Opened the link in several new tabs/windows using e10s
- Opened the link in several new tabs/windows using non-e10s
- Opened the link in several new private windows/tabs
Status: RESOLVED → VERIFIED
status-firefox41: fixed → verified
status-firefox42: fixed → verified
status-firefox43: fixed → verified
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.