1340482 - Shape field not traced for non-native shaped objects

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

[Jon Coppeard (:jonco)]

Attachment: trace-object-shape

Since we introduced ShapedObject, JSObject::traceChildren() doesn't trace the shape if the object is shaped but not native.  This might cause UAF if a shape is swept while still in use (although mostly objects are marked by a different path), or if such an object is moved by compacting GC.

Comment 1

[Jon Coppeard (:jonco)]

Comment on attachment 8838474 [details] [diff] [review]
trace-object-shape

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

Very difficult.

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?

Everything back to 50.

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

Bug 1284634.

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

Should be trivial.

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

Unlikely.

Comment 2

[Al Billings [:abillings - ex-MoCo]]

We've had our last beta. I think this is too late for 52, which affects sec-approval, but it is a small change. I'm asking release management for their opinion.

Comment 3

[Julien Cristau [:jcristau]]

Let's aim to fix this in 53.

Comment 4

[Al Billings [:abillings - ex-MoCo]]

sec-approval+ for checkin on 3/21, two weeks into the next cycle. We're a week from final release of 52 right now.

Comment 5

[Andrew McCreight [:mccr8]]

[Tracking Requested - why for this release]: sec-high

Comment 6

[Gerry Chang [:gchang]]

Track 54+ as sec-high.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/846948a41b6c

Please request Aurora/Beta/ESR52 approval on this as soon as possible.

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/846948a41b6c

Comment 9

[Jon Coppeard (:jonco)]

Comment on attachment 8838474 [details] [diff] [review]
trace-object-shape

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1284634.
[User impact if declined]: Possible crash / security vulnerability.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: This is a simple change to trace objects' shapes in some circumstances when it wasn't being traced before.
[String changes made/needed]: None.

Comment 10

[Jon Coppeard (:jonco)]

Comment on attachment 8838474 [details] [diff] [review]
trace-object-shape

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: sec-high bug.
User impact if declined: Possible crash / security vulnerability.
Fix Landed on Version: 55.
Risk to taking this patch (and alternatives if risky): Low.
String or UUID changes made by this patch: None.

Comment 11

[Marcia Knous [:marcia]]

Tracking 55+, as well as for esr52.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Shouldn't the esr52 tracking be 53+ since that's the corresponding Fx release we're hoping to ship this with?

Comment 13

[Marcia Knous [:marcia]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #12)
> Shouldn't the esr52 tracking be 53+ since that's the corresponding Fx
> release we're hoping to ship this with?

Yes, sorry I will correct that.

Comment 14

[Gerry Chang [:gchang]]

Comment on attachment 8838474 [details] [diff] [review]
trace-object-shape

Fix a  sec-high. Aurora54+ & Beta53+.

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/0fff3bdd02a7

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3d0b8766a2a5

Comment 17

[Julien Cristau [:jcristau]]

Comment on attachment 8838474 [details] [diff] [review]
trace-object-shape

sec-high uaf fix for esr52

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/512604631b23

Comment 19

[Andrei Vaida [:avaida]]

Setting qe-verify- based on Jon's assessment on manual testing needs and the fact that this fix has automated coverage (see Comment 17).