Closed Bug 1394493 Opened 3 years ago Closed 3 years ago

Assertion failure: v.isSymbol(), at js/src/vm/Interpreter.cpp:4392 or Assertion failure: v.isUndefined(), at js/src/jsstr.cpp:3593

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox-esr52 --- wontfix
firefox55 --- wontfix
firefox56 --- fixed
firefox57 --- fixed

People

(Reporter: decoder, Assigned: jonco)

References

Details

(4 keywords, Whiteboard: [jsbugmon:update])

Attachments

(1 file)

The following testcase crashes on mozilla-central revision d10c97627b51 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe):

function evalModuleAndCheck(source, expected) {
    let m = parseModule(source);
    getModuleEnvironmentValue(m, "r").toString()
}
evalModuleAndCheck("export let r = y; y = 4;");



Backtrace:

 received signal SIGSEGV, Segmentation fault.
0x000000000052bfa8 in js::GetProperty (cx=0x7ffff6924000, v=..., name=..., vp=...) at js/src/vm/Interpreter.cpp:4392
#0  0x000000000052bfa8 in js::GetProperty (cx=0x7ffff6924000, v=..., name=..., vp=...) at js/src/vm/Interpreter.cpp:4392
#1  0x0000000000531103 in GetPropertyOperation (vp=..., lval=..., pc=<optimized out>, script=..., fp=<optimized out>, cx=<optimized out>) at js/src/vm/Interpreter.cpp:192
#2  Interpret (cx=0x7ffff6924000, state=...) at js/src/vm/Interpreter.cpp:2784
#3  0x000000000053da33 in js::RunScript (cx=0x7ffff6924000, state=...) at js/src/vm/Interpreter.cpp:409
[...]
#12 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:8592
rax	0x0	0
rbx	0x7ffff6924000	140737330167808
rcx	0x7ffff6c28a2d	140737333332525
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffcc50	140737488342096
rsp	0x7fffffffcbb0	140737488341936
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4740	140737354024768
r10	0x58	88
r11	0x7ffff6b9f750	140737332770640
r12	0x7fffffffd0a0	140737488343200
r13	0x7ffff4280140	140737289650496
r14	0x7fffffffd0e0	140737488343264
r15	0x7ffff6924020	140737330167840
rip	0x52bfa8 <js::GetProperty(JSContext*, JS::Handle<JS::Value>, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>)+776>
=> 0x52bfa8 <js::GetProperty(JSContext*, JS::Handle<JS::Value>, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>)+776>:	movl   $0x0,0x0
   0x52bfb3 <js::GetProperty(JSContext*, JS::Handle<JS::Value>, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>)+787>:	ud2


Marking s-s because both assertions can indicate security-sensitive type confusions.
anba could this be from your module changes?

Just a wild guess, JSBugMon bisection results would be good :)
(In reply to Jan de Mooij [:jandem] from comment #1)
> anba could this be from your module changes?
> 
> Just a wild guess, JSBugMon bisection results would be good :)

No, I've recently only touched ModuleNamespaceObject, but didn't tinker with ModuleEnvironmentObject.

GetModuleEnvironmentValue is probably just missing an uninitialized-lexical check. With the following patch applied, the test case no longer asserts:

diff --git a/js/src/builtin/TestingFunctions.cpp b/js/src/builtin/TestingFunctions.cpp
--- a/js/src/builtin/TestingFunctions.cpp
+++ b/js/src/builtin/TestingFunctions.cpp
@@ -3974,17 +3974,23 @@ GetModuleEnvironmentValue(JSContext* cx,
     }
 
     RootedModuleEnvironmentObject env(cx, GetModuleEnvironment(cx, args[0]));
     RootedString name(cx, args[1].toString());
     RootedId id(cx);
     if (!JS_StringToId(cx, name, &id))
         return false;
 
-    return GetProperty(cx, env, env, id, args.rval());
+    if (!GetProperty(cx, env, env, id, args.rval()))
+        return false;
+    if (args.rval().isMagic(JS_UNINITIALIZED_LEXICAL)) {
+        ReportRuntimeLexicalError(cx, JSMSG_UNINITIALIZED_LEXICAL, id);
+        return false;
+    }
+    return true;
 }
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/1ac68e528d12
user:        Jon Coppeard
date:        Thu Oct 08 10:49:49 2015 +0100
summary:     Bug 1209107 - Only expose module environment object through testing functions r=shu

Jon, is bug 1209107 a likely regressor?
Blocks: 1209107
Flags: needinfo?(jcoppeard)
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
André is correct.  

This is just a problem with the getModuleEnvironmentValue() testing function, not a security issue.
Group: javascript-core-security
Flags: needinfo?(jcoppeard)
Assignee: nobody → jcoppeard
Attachment #8903085 - Flags: review?(arai.unmht)
Attachment #8903085 - Flags: review?(arai.unmht) → review+
Blocks: 1394791
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ad07e576ee7
Check for uninitialised lexicals in getModuleEnvironmentValue() r=arai
Duplicate of this bug: 1394791
https://hg.mozilla.org/mozilla-central/rev/8ad07e576ee7
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Flags: in-testsuite+
Whiteboard: [jsbugmon:update] → [jsbugmon:update][checkin-needed-beta]
https://hg.mozilla.org/releases/mozilla-beta/rev/06408f33c66c
Whiteboard: [jsbugmon:update][checkin-needed-beta] → [jsbugmon:update]
You need to log in before you can comment on or make changes to this bug.