Assertion failure: next == JSOp::CheckThis || next == JSOp::CheckReturn || next == JSOp::CheckThisReinit || next == JSOp::CheckLexical, at vm/Interpreter.cpp:3715 or Assertion failure: v.isSymbol() || v.isBigInt(), at jsnum.cpp:1944
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
People
(Reporter: anba, Assigned: anba, NeedInfo)
References
(Regression)
Details
(Keywords: regression, sec-high, testcase, Whiteboard: [sec-survey][adv-main85+r][adv-esr78.7+r])
Attachments
(2 files)
48 bytes,
text/x-phabricator-request
|
dveditz
:
approval-mozilla-beta+
RyanVM
:
approval-mozilla-esr78+
dveditz
:
sec-approval+
|
Details | Review |
48 bytes,
text/x-phabricator-request
|
Details | Review |
+++ This bug was initially created as a clone of Bug #1684020 +++
{
var a = null;
a?.[b];
!!b;
let b;
}
Crashes the same way as bug 1684020.
Assignee | ||
Comment 1•4 years ago
|
||
Updated•4 years ago
|
Assignee | ||
Comment 2•4 years ago
|
||
Depends on D100883
Assignee | ||
Comment 3•4 years ago
|
||
Comment on attachment 9195591 [details]
Bug 1685260: Add TDZCheckCache. r=yulia!
Beta/Release Uplift Approval Request
- User impact if declined: The
JS_UNINITIALIZED_LEXICAL
magic value is exposed to user code, which can lead to wrong JavaScript behaviour and crashes. It's unclear if this issue can be used for anything worse and becauseJS::Value
is used in many places in SpiderMonkey/Gecko, it's impossible to check every line of code processingJS::Value
. - Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): Single line patch to use a fresh
TDZCheckCache
.TDZCheckCache
keeps track of definitely accessed variables and creating a new one is a safe operation, for example for every arm of anif
-statement, a newTDZCheckCache
is used. - String changes made/needed:
ESR Uplift Approval Request
- If this is not a sec:{high,crit} bug, please state case for ESR consideration:
- User impact if declined: See beta uplift request.
- Fix Landed on Version:
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): See beta uplift request.
- String or UUID changes made by this patch:
Comment 4•4 years ago
|
||
Please request sec-approval on this patch.
Assignee | ||
Comment 5•4 years ago
|
||
Comment on attachment 9195591 [details]
Bug 1685260: Add TDZCheckCache. r=yulia!
Security Approval Request
- How easily could an exploit be constructed based on the patch?: Probably easily because it's a single line change. That line added
TDZCheckCache
which makes it also easy to see where we had a problem: "TDZ" is the abbreviation for "temporal dead zone", which refers to the zone before a declaration of a lexical binding (let
,const
, etc.) where any access to a lexical binding must throw aReferenceError
.
So when taking into account where the line was added (bytecode emitter for optional chaining) and what was added (TDZCheckCache
), it should be easy to determine what was broken 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 (74+)
- If not all supported branches, which bug introduced the flaw?: None
- Do you have backports for the affected branches?: Yes
- If not, how different, hard to create, and risky will they be?:
- How likely is this patch to cause regressions; how much testing does it need?: Unlikely, see comment #3.
Updated•4 years ago
|
Updated•4 years ago
|
Comment 6•4 years ago
|
||
Comment on attachment 9195591 [details]
Bug 1685260: Add TDZCheckCache. r=yulia!
sec-approval+, a=dveditz for landing on beta. Please don't land the test until after we ship the fix (in-testsuite?
flag set)
Comment 7•4 years ago
|
||
Comment 8•4 years ago
|
||
Comment 9•4 years ago
|
||
Comment on attachment 9195591 [details]
Bug 1685260: Add TDZCheckCache. r=yulia!
Approved for 78.7esr.
Comment 10•4 years ago
|
||
uplift |
Comment 11•4 years ago
|
||
uplift |
Comment 12•4 years ago
|
||
As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.
Please visit this google form to reply.
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Comment 13•3 years ago
|
||
Updated•3 years ago
|
Comment 14•3 years ago
|
||
bugherder |
Updated•3 years ago
|
Description
•