1639590 - (CVE-2020-12406) Fix GetOwnPropertyPure

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Iain Ireland [:iain]]

In bug 1620193 we fixed a mistake that crept in during unboxed objects removal, where we didn't check that an object was native before calling as<NativeObject> on it. (Fortunately proxy objects couldn't reach that point, so it only affected typed objects, which are nightly-only.)

That patch was flagged in a mozregression for bug 1638706. I've convinced myself that the mozregression was wrong, but while I was staring at this code I noticed that there was a second instance of the same bug in the following function.

Like bug 1620193, this bug should only affect nightly.

Comment 1

[Iain Ireland [:iain]]

Attachment: Bug 1639590: Finish removing unboxed objects code r=mgaudet

Comment 2

[Matthew Gaudet (he/him) [:mgaudet]]

I think the tracking flags are wrong, and I've reset them based on the original regressing bug (also set).

You can see the old isNative check here: https://hg.mozilla.org/integration/autoland/file/0330a759e399/js/src/vm/JSObject.cpp#l2671

Comment 3

[Ryan VanderMeulen [:RyanVM]]

We're building the RC builds for 77/68.9esr early next week, so please request sec-approval on this patch ASAP. Looks like the patch applies cleanly as-is to Beta and ESR68, so feel free to go ahead with those approval requests also.

Comment 4

[Iain Ireland [:iain]]

Comment on attachment 9150467 [details]
Bug 1639590: Finish removing unboxed objects code r=mgaudet

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Unclear. This bug was identified by looking at the code, not based on a failing testcase. The incorrect function only has two callers, so it's possible that it can't be exploited.
• 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?: All
• If not all supported branches, which bug introduced the flaw?: Bug 1505574
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?: This patch applies cleanly to all supported branches.
• How likely is this patch to cause regressions; how much testing does it need?: Very unlikely. Any code affected by this patch would trigger a debug assert. Also, like the equivalent bug in bug 1620193, this should only be possible to trigger using typed objects, which are nightly only.

Comment 5

[Daniel Veditz [:dveditz]]

Comment on attachment 9150467 [details]
Bug 1639590: Finish removing unboxed objects code r=mgaudet

sec-approval+

Comment 6

[Daniel Veditz [:dveditz]]

Comment on attachment 9150467 [details]
Bug 1639590: Finish removing unboxed objects code r=mgaudet

we're going to take this in FIrefox 77, which means landing on mozilla-release, and also landing on the ESR branch (no RC for ESR yet so I don't think there's any special sub-branch). a=dveditz for branches after checking w/Ryan and Pascal

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/autoland/rev/8f7708eeb4ef46d23dabe2f64db46a7e02bd168c

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/9ad767398b408a09b8ef9512b86fd3fc97832888
https://hg.mozilla.org/releases/mozilla-esr68/rev/335f6c486ca455444ae6790a52568eae2497704b

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/8f7708eeb4ef

Comment 10

[Frederik Braun [:freddy]]

Attachment: advisory.txt

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

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.