Closed Bug 1294767 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: JavaScript: GC, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox50 --- wontfix
firefox51 --- wontfix
firefox52 --- fixed

People

(Reporter: jonco, Assigned: jonco)

References

Details

(Keywords: sec-audit, Whiteboard: [adv-main52-])

Attachments

(1 file)

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.
Keywords: sec-audit
Blocks: 1220385
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.
Attachment #8792552 - Flags: review?(jdemooij)
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.
Attachment #8792552 - Flags: review?(jdemooij) → review+
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.
Group: javascript-core-security
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed8f6b83ebed
Add assertions that type sets do not contain cross compartment pointers r=jandem
https://hg.mozilla.org/mozilla-central/rev/ed8f6b83ebed
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Should this land on 51 as well or can it ride the trains?
Flags: needinfo?(jcoppeard)
This doesn't seem to have shown up any problems.  I'd say just let it ride.
Flags: needinfo?(jcoppeard)
Whiteboard: [adv-main52-]
You need to log in before you can comment on or make changes to this bug.