If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Assertion failure: Argument check fail., at js/src/jit/MacroAssembler.cpp:1445 with OOM

RESOLVED FIXED in Firefox 45

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: decoder, Assigned: jandem)

Tracking

(Blocks: 2 bugs, 4 keywords)

Trunk
mozilla47
x86_64
Linux
assertion, regression, sec-high, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox44 wontfix, firefox45+ fixed, firefox46+ fixed, firefox47+ fixed, firefox-esr3845+ fixed, firefox-esr45 fixed)

Details

(Whiteboard: [jsbugmon:][adv-main45+][adv-esr38.7+][post-critsmash-triage])

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
The following testcase crashes on mozilla-central revision 5f7c184ccd80 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug, run with --fuzzing-safe --thread-count=2 --ion-offthread-compile=off --baseline-eager --ion-eager):

lfBuffer = `

objectEmulatingUndefined = function() 0;  
new Date().toLocaleString("enUS", {});
oomTest(()=> 0({thisprops: gc() && addDebuggee.enabled}));
`
lfBuffer = lfBuffer.split("\n");
lfCodeBuffer = 0;

for (i = 0; i < 10; ++i) {
    line = lfBuffer.shift()
    lfCodeBuffer += line + "\n"
    loadFile(lfCodeBuffer)
    function loadFile(lfVarx) {
                eval(lfVarx)
    }
}



Backtrace:

Program received signal SIGTRAP, Trace/breakpoint trap.
0x00007ffff7f87c00 in ?? ()
#0  0x00007ffff7f87c00 in ?? ()
#1  0x00007ffff7f24f7d in ?? ()
#2  0x0000000000000a00 in ?? ()
#3  0x00007ffff7e8c651 in ?? ()
#4  0x0000000000000000 in ?? ()
rax	0x7ffff7eca250	140737352868432
rbx	0x7ffff7f87a49	140737353644617
rcx	0xfffafffff7ea60d0	-1407375019188016
rdx	0x7ffff4800680	140737295419008
rsi	0x0	0
rdi	0x7fffffff4bb0	140737488309168
rbp	0x4	4
rsp	0x7fffffff4db0	140737488309680
r8	0x6	6
r9	0x0	0
r10	0xc8e65ca2	3370540194
r11	0x7ffff6c27960	140737333328224
r12	0x73	115
r13	0x7fffffff5580	140737488311680
r14	0x0	0
r15	0x7ffff48005c0	140737295418816
rip	0x7ffff7f87c00	140737353645056
=> 0x7ffff7f87c00:	movabs $0x7ffff695d1f8,%r11
   0x7ffff7f87c0a:	cmp    %rsp,(%r11)


This OOM is quite fragile, just addings prints anywhere in the code make the crash go away. I've seen this before but only this time I was able to extract a proper test.

Updated

2 years ago
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:bisect]

Comment 1

2 years ago
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Updated

2 years ago
Whiteboard: [jsbugmon:bisect] → [jsbugmon:]
(Assignee)

Updated

2 years ago
Flags: needinfo?(jdemooij)
(Assignee)

Comment 2

2 years ago
Hiding this for now; potential type inference issue.
Group: javascript-core-security
(Assignee)

Comment 3

2 years ago
Created attachment 8712134 [details] [diff] [review]
Patch

Great catch fuzzers. It took me a while to track this down.

Scripts have a `hasFreezeConstraints` flag, and if we OOM while copying type constraints, we need to reset that flag or we risk losing a freeze constraint.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(jdemooij)
Attachment #8712134 - Flags: review?(bhackett1024)
(Assignee)

Comment 4

2 years ago
It's hard to trigger/exploit this bug, but it might be possible, so I'll rate this sec-high.
Keywords: sec-high
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1238577
Attachment #8712134 - Flags: review?(bhackett1024) → review+
(Assignee)

Comment 6

2 years ago
Comment on attachment 8712134 [details] [diff] [review]
Patch

[Security approval request comment]
* How easily could an exploit be constructed based on the patch?
Pretty difficult, but we've had OOM bugs exploited before.

* 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?
All.

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

* How likely is this patch to cause regressions; how much testing does it need?
Unlikely to cause regressions; it also only affects OOM cases.
Attachment #8712134 - Flags: sec-approval?
sec-approval+ for checkin on February 9. 

Please make nominations for Aurora, Beta, and ESR38 so we can have it (or a derived patch) do in following Mozilla Central.
status-firefox44: --- → wontfix
status-firefox45: --- → affected
status-firefox47: --- → affected
status-firefox-esr38: --- → affected
status-firefox-esr45: --- → affected
tracking-firefox45: --- → +
tracking-firefox46: --- → +
tracking-firefox47: --- → +
tracking-firefox-esr38: --- → 45+
Whiteboard: [jsbugmon:] → [jsbugmon:][checkin on 2/9]
Attachment #8712134 - Flags: sec-approval? → sec-approval+
Jan, could you fill the uplift requests? Thanks
Flags: needinfo?(jdemooij)
Keywords: checkin-needed
Whiteboard: [jsbugmon:][checkin on 2/9] → [jsbugmon:]
https://hg.mozilla.org/integration/mozilla-inbound/rev/53416fc6f2ce
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/53416fc6f2ce
status-firefox47: affected → fixed
Target Milestone: --- → mozilla47

Updated

2 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
(Assignee)

Comment 11

2 years ago
Comment on attachment 8712134 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: Unknown.
[User impact if declined]: Crashes, security bugs.
[Describe test coverage new/current, TreeHerder]: Landed on m-c. Fixed the reported test.
[Risks and why]: Low risk; only affects a particular OOM situation.
[String/UUID change made/needed]: None.
Flags: needinfo?(jdemooij)
Attachment #8712134 - Flags: approval-mozilla-esr38?
Attachment #8712134 - Flags: approval-mozilla-beta?
Attachment #8712134 - Flags: approval-mozilla-aurora?
Comment on attachment 8712134 [details] [diff] [review]
Patch

Fix a sec high, taking it.
Should be in 45 beta 5.
Attachment #8712134 - Flags: approval-mozilla-esr38?
Attachment #8712134 - Flags: approval-mozilla-esr38+
Attachment #8712134 - Flags: approval-mozilla-beta?
Attachment #8712134 - Flags: approval-mozilla-beta+
Attachment #8712134 - Flags: approval-mozilla-aurora?
Attachment #8712134 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/81b7b203feb1
status-firefox46: affected → fixed
https://hg.mozilla.org/releases/mozilla-beta/rev/bfb97f135a93
status-firefox45: affected → fixed
Group: javascript-core-security → core-security-release
Wes, one more that also needs to land on ESR38. Thanks!
Flags: needinfo?(wkocher)
https://hg.mozilla.org/releases/mozilla-esr38/rev/2839062f84fb
status-firefox-esr38: affected → fixed
Flags: needinfo?(wkocher)
status-firefox-esr45: affected → fixed
Whiteboard: [jsbugmon:] → [jsbugmon:][adv-main45+][adv-esr38.7+]
Whiteboard: [jsbugmon:][adv-main45+][adv-esr38.7+] → [jsbugmon:][adv-main45+][adv-esr38.7+][post-critsmash-triage]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.