Closed Bug 1189744 Opened 5 years ago Closed 5 years ago

Crash c0000005_xul.dll!js::CallJSGetterOp

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla43
Tracking Status
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

People

(Reporter: cbook, Assigned: jorendorff)

References

()

Details

(Keywords: crash, regression, sec-critical)

Attachments

(2 files)

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.
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
Flags: needinfo?(jdemooij) → needinfo?(jorendorff)
Tracking for 41, 42 because regression.
Assignee: nobody → jorendorff
Status: NEW → ASSIGNED
Thanks for looking into this, Jan. Definitely needs to land on the branch.
Flags: needinfo?(jorendorff)
Attachment #8643362 - Flags: review?(jdemooij) → review+
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+
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 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+
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
Closed: 5 years ago
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.
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
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.