The following testcase asserts on mozilla-central revision 1b0b56afa33a (run with -m -n -a): Object.prototype.apply = Function.prototype.apply; function testApplyCallHelper(f) { var i = 0; i.apply(this,[,,2]) } testApplyCallHelper()

Opt-build shows: ==52813== Invalid read of size 8 ==52813== at 0x4BA2E0: js::DecompileValueGenerator(JSContext*, int, JS::Handle<JS::Value>, JS::Handle<JSString*>, int) (jsopcode.cpp:5823) ==52813== by 0x437C08: js_ReportValueErrorFlags(JSContext*, unsigned int, unsigned int, int, JS::Handle<JS::Value>, JS::Handle<JSString*>, char const*, char const*) (jscntxt.cpp:958) ==52813== by 0x490E80: js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) (jsinterp.cpp:242) ==52813== by 0x634D7B: js::mjit::stubs::SlowCall(js::VMFrame&, unsigned int) (InvokeHelpers.cpp:133) ==52813== by 0x621DB7: js::mjit::ic::NativeCall(js::VMFrame&, js::mjit::ic::CallICInfo*) (MonoIC.cpp:1008) ==52813== by 0x403AAA6: ??? ==52813== by 0x5B64B6: js::mjit::EnterMethodJIT(JSContext*, js::StackFrame*, void*, JS::Value*, bool) (MethodJIT.cpp:1016) ==52813== by 0x5B665D: js::mjit::JaegerShot(JSContext*, bool) (MethodJIT.cpp:1074) ==52813== by 0x48F555: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinterp.cpp:2438) ==52813== by 0x5B64F3: js::mjit::EnterMethodJIT(JSContext*, js::StackFrame*, void*, JS::Value*, bool) (MethodJIT.cpp:1043) ==52813== by 0x5B665D: js::mjit::JaegerShot(JSContext*, bool) (MethodJIT.cpp:1074) ==52813== by 0x490C76: js::RunScript(JSContext*, JSScript*, js::StackFrame*) (jsinterp.cpp:298) ==52813== Address 0x6091618 is 8 bytes before a block of size 48 alloc'd ==52813== at 0x4C28FAC: malloc (vg_replace_malloc.c:236) ==52813== by 0x4BA28B: js::DecompileValueGenerator(JSContext*, int, JS::Handle<JS::Value>, JS::Handle<JSString*>, int) (Utility.h:157) ==52813== by 0x437C08: js_ReportValueErrorFlags(JSContext*, unsigned int, unsigned int, int, JS::Handle<JS::Value>, JS::Handle<JSString*>, char const*, char const*) (jscntxt.cpp:958) ==52813== by 0x490E80: js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) (jsinterp.cpp:242) ==52813== by 0x634D7B: js::mjit::stubs::SlowCall(js::VMFrame&, unsigned int) (InvokeHelpers.cpp:133) ==52813== by 0x621DB7: js::mjit::ic::NativeCall(js::VMFrame&, js::mjit::ic::CallICInfo*) (MonoIC.cpp:1008) ==52813== by 0x403AAA6: ??? ==52813== by 0x5B64B6: js::mjit::EnterMethodJIT(JSContext*, js::StackFrame*, void*, JS::Value*, bool) (MethodJIT.cpp:1016) ==52813== by 0x5B665D: js::mjit::JaegerShot(JSContext*, bool) (MethodJIT.cpp:1074) ==52813== by 0x48F555: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinterp.cpp:2438) ==52813== by 0x5B64F3: js::mjit::EnterMethodJIT(JSContext*, js::StackFrame*, void*, JS::Value*, bool) (MethodJIT.cpp:1043) ==52813== by 0x5B665D: js::mjit::JaegerShot(JSContext*, bool) (MethodJIT.cpp:1074) S-s due to possible out-of-bounds read with unknown impact.

JSBugMon: Bisection requested, result: autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: 103051:bf07c6253287 user: Brian Hackett date: Wed Aug 22 12:28:34 2012 -0600 summary: Allow purging analysis-temporary while retaining jitcode, bug 778724. r=luke

Brian and Luke, can you have a look per comment 2?

Assuming sec-high due to invalid read.

Assigning to Brian per comment 2, feel free to reassign if you're not a good owner.

I can still reproduce on a recent m-c tip changeset c8a1314aa449 with -a on Mac 10.8 64-bit js shell with determinism disabled.

JSBugMon: Cannot process bug: Unknown exception (check manually)

JSBugMon: The testcase found in this bug no longer reproduces (tried revision 712eca11a04e).

JSBugMon: Fix Bisection requested, result: autoBisect shows this is probably related to the following changeset: The first good revision is: changeset: 118999:39885ae3a597 user: Brendan Eich date: Tue Jan 15 18:17:50 2013 -0800 summary: Bug 810525 - unregress DecompileValueGenerator change to handle object literal reference bases (r=jandem). This iteration took 107.488 seconds to run.

Brendan, did the fix in comment 9 fix this security issue?

Asking Jan instead for comment 9/10.

Yeah, that bug added a missing bounds check and avoids using the decompiler if JM lowered fun.apply (the testcase in comment 0 uses fun.apply).

Added the test: https://hg.mozilla.org/integration/mozilla-inbound/rev/eaf0e938db6d

JSBugMon: This bug has been automatically verified fixed.

Should have checked first that this doesn't need uplifting, but it seems it does... Checking affected branches now.

JSBugMon: The testcase found in this bug does not reproduce on branch mozilla-aurora (tried revision bb20a6500778). JSBugMon: This bug has been automatically confirmed to be still valid on branch mozilla-beta (reproduced on revision ceaba6121750). JSBugMon: This bug has been automatically confirmed to be still valid on branch mozilla-release (reproduced on revision 1f29514cd560). JSBugMon: This bug has been automatically confirmed to be still valid on branch mozilla-esr17 (reproduced on revision 659ce2b0bb83).

Nominating for affected branches based on comment 17.

Brian is this also 17/18 affected? What is the proposed fix for branches and what is the risk? This doesn't look necessary for FF20 given that it looks to be a long-standing issue so please let us know if we're not reading this correctly.

esr17 is affected by comment 17. I nominated this because dveditz told me to do so, after I accidentially landed the test already now, although the fix hadn't been uplifted.

If we take the fix in bug 810525 then we also need the fix for security bug 831846 and compiler warning bug 831205, and will introduce unfixed bug 850949. It's also possible that the changes in bug 810525 will cause additional problems when backported since we don't know if they depend on other changes between Firefox 17 and 21.

The testcase does not crash a release ESR 17.0.4, we can hope that people won't discover the underlying issue merely from the checked-in test.

(In reply to Daniel Veditz [:dveditz] from comment #22) > The testcase does not crash a release ESR 17.0.4, we can hope that people > won't discover the underlying issue merely from the checked-in test. Are you sure you run it the right way? JSBugMon confirmed this on ESR17.

(In reply to lsblakk@mozilla.com from comment #19) > Brian is this also 17/18 affected? What is the proposed fix for branches and > what is the risk? This doesn't look necessary for FF20 given that it looks > to be a long-standing issue so please let us know if we're not reading this > correctly. Jan would know more. The original blame for this bug is wrong, bug 778724 doesn't have anything to do with this failure.

(In reply to Christian Holler (:decoder) from comment #14) > Added the test: > > https://hg.mozilla.org/integration/mozilla-inbound/rev/eaf0e938db6d https://hg.mozilla.org/mozilla-central/rev/eaf0e938db6d

So is this really fixed on 21? Also we should uplift to b2g18 due to security level.

jandem makes more sense as an assignee

jandem - can bug 810525 be uplifted to ESR17/B2G18?

I think this is the right fix for esr17. It falls back to the stack search if spindex is out-of-range. Note that we already have the exact same check on mozilla-central, so this should be very low-risk.

Please nominate for uplift to ESR/B2G when you get the chance. Thanks!

Comment on attachment 732395 [details] [diff] [review] Patch for esr17 [Approval Request Comment] Bug caused by (feature/regressing bug #): Bisection points to bug 778724, but likely existed before that. User impact if declined: Security issues, crashes. Testing completed: Exact same code is on beta, aurora and m-c. Risk to taking this patch (and alternatives if risky): Very low. String or UUID changes made by this patch: None. [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: User impact if declined: Security issues, crashes. Fix Landed on Version: Exact same code is on beta, aurora and m-c. Risk to taking this patch (and alternatives if risky): Very low. String or UUID changes made by this patch: None.

https://hg.mozilla.org/releases/mozilla-b2g18/rev/e92cfa71f35f https://hg.mozilla.org/releases/mozilla-esr17/rev/558830cbccd9

Reproduced on FF17.0.5esr Confirmed fixed on FF17.0.6esr candidate build 1 Confirmed fixed on FF21 candidate build 2 Confirmed fixed on FF22 2013-05-09 Confirmed fixed on FF23 2013-05-09