Closed Bug 809295 Opened 7 years ago Closed 7 years ago

Do a better job handling failure in JSCompartment::wrap

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla19
Tracking Status
firefox18 --- affected
firefox19 --- fixed
firefox-esr10 --- unaffected
firefox-esr17 --- unaffected
b2g18 --- wontfix

People

(Reporter: billm, Assigned: billm)

References

Details

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

Attachments

(1 file)

Attached patch patchSplinter Review
Since bug 793904, we're supposed to crash intentionally if a brain transplant fails. However, I realized that I missed a few cases where a brain transplant happens, so this patch adds some more MOZ_CRASH calls.

However, these extra calls make us crash during a JS reftest that uses up lots of stack and causes the recursion check in JSCompartment::wrap to fail. I considered taking out the recursion check, but it still seems useful given that PreCreate hooks could conceivably cause recursive wrap() calls. So instead I just gave the wrap() call a little extra C stack to work with than the rest of the engine gets. I think this makes sense, given how important it is that brain transplants succeed.

Note that C stack limit we use is a made-up number that's conservative, so we should have some leeway to exceed it as long as we don't go too far.
Attachment #679003 - Flags: review?(luke)
I'm going to mark this as sec-other, as bug 793904 wasn't a security bug. Feel free to mark it high or crit if you want this to be tracked more by CritSmash.
Keywords: sec-other
Comment on attachment 679003 [details] [diff] [review]
patch

Makes sense; nice compromise.
Attachment #679003 - Flags: review?(luke) → review+
https://hg.mozilla.org/mozilla-central/rev/83fe0193339b
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Whiteboard: [adv-main19+]
Group: core-security
You need to log in before you can comment on or make changes to this bug.