1294767 - Add assertions that type inference data structures don't contain cross compartment pointers

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

[Jon Coppeard (:jonco)]

Following bug 1294241, we should investigate adding assertions that type sets don't contain cross compartment pointers as this is a sure-fire way get UAF bugs.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: bug1294767-type-set-asserts

Here's a patch to add some compartment checking asserts to type inference.  The aim of this is to prevent UAF bugs like that referenced above and also to hopefully shed some light on bug 1220385.

Comment 2

[Jan de Mooij [:jandem]]

Comment on attachment 8792552 [details] [diff] [review]
bug1294767-type-set-asserts

Review of attachment 8792552 [details] [diff] [review]:
-----------------------------------------------------------------

Nice.

::: js/src/vm/TypeInference-inl.h
@@ +1017,5 @@
>      return nullptr;
>  }
>  
> +inline JSCompartment*
> +TypeSet::maybeCompartment()

Nit: maybe move to the cpp file, as it's only called there AFAICS and it's pretty large.

Comment 3

[Jon Coppeard (:jonco)]

I don't think this needs to be s-s as it only adds asserts and doesn't highlight any specific vulnerability.  Try is green with this patch.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed8f6b83ebed
Add assertions that type sets do not contain cross compartment pointers r=jandem

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ed8f6b83ebed

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Should this land on 51 as well or can it ride the trains?

Comment 7

[Jon Coppeard (:jonco)]

This doesn't seem to have shown up any problems.  I'd say just let it ride.