1928654 - Assertion failure: isDouble(), at js/Value.h:1070

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Nils Bars]

Steps to reproduce:

Version: ff95c781c335e83ba8f5be401e479201fe28a3f5
Args: js --fuzzing-safe <test-case>
Test case:

a = function() {
  getOwnPropertyNames = Object.getOwnPropertyNames
  function b(c) {
    try {
      d = getOwnPropertyNames(c)
      for (e = 0;; e++) {
        f = d[e]
        c[f]
      }
    } catch {
    }
  }
  return b
}()
g = parseModule("{}", "", "json")
a(g)

Actual results:

Assertion failure: isDouble(), at js/Value.h:1070

#0 0x563f6137ce81 in JS::Value::toPrivate() const reproducebuild/dist/include/js/Value.h:1070:5
#1 0x563f6137ce81 in js::ModuleObject::cyclicModuleFields() js/src/builtin/ModuleObject.cpp:921:55
#2 0x563f6138065c in js::ModuleObject::cyclicModuleFields() const js/src/builtin/ModuleObject.cpp:926:43
#3 0x563f6138065c in js::ModuleObject::maybeEvaluationError() const js/src/builtin/ModuleObject.cpp:1335:10
#4 0x563f611dde9f in bool ShellModuleWrapperGetter<js::shell::ShellModuleObjectWrapper, JS::Value (*)(js::ModuleObject*), bool (*)(JSContext*, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>)>(JSContext*, JS::CallArgs const&, JS::Value (*)(js::ModuleObject*), bool (*)(JSContext*, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>)) js/src/shell/ShellModuleObjectWrapper.cpp:270:33
#5 0x563f61195799 in ShellModuleObjectWrapper_maybeEvaluationErrorGetter_impl(JSContext*, JS::CallArgs const&) js/src/shell/ShellModuleObjectWrapper.cpp:443:1
#6 0x563f61195799 in bool JS::CallNonGenericMethod<&IsShellModuleObjectWrapper(JS::Handle<JS::Value>), &ShellModuleObjectWrapper_maybeEvaluationErrorGetter_impl(JSContext*, JS::CallArgs const&)>(JSContext*, JS::CallArgs const&) reproducebuild/dist/include/js/CallNonGenericMethod.h:103:12
#7 0x563f61195799 in ShellModuleObjectWrapper_maybeEvaluationErrorGetter(JSContext*, unsigned int, JS::Value*) js/src/shell/ShellModuleObjectWrapper.cpp:443:1
#8 0x563f61274f6e in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) js/src/vm/Interpreter.cpp:528:13
#9 0x563f612741cf in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) js/src/vm/Interpreter.cpp:624:12
#10 0x563f612760ca in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) js/src/vm/Interpreter.cpp:723:8
#11 0x563f61277dbf in js::CallGetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) js/src/vm/Interpreter.cpp:845:10
#12 0x563f61708217 in CallGetter(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<JS::Value>, JS::Handle<JS::PropertyKey>, js::PropertyInfoBase<unsigned int>, JS::MutableHandle<JS::Value>) js/src/vm/NativeObject.cpp:2149:12
#13 0x563f61708217 in bool GetExistingProperty<(js::AllowGC)1>(JSContext*, js::MaybeRooted<JS::Value, (js::AllowGC)1>::HandleType, js::MaybeRooted<js::NativeObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::PropertyKey, (js::AllowGC)1>::HandleType, js::PropertyInfoBase<unsigned int>, js::MaybeRooted<JS::Value, (js::AllowGC)1>::MutableHandleType) js/src/vm/NativeObject.cpp:2177:12
#14 0x563f61708e17 in bool NativeGetPropertyInline<(js::AllowGC)1>(JSContext*, js::MaybeRooted<js::NativeObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::Value, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::PropertyKey, (js::AllowGC)1>::HandleType, IsNameLookup, js::MaybeRooted<JS::Value, (js::AllowGC)1>::MutableHandleType) js/src/vm/NativeObject.cpp:2330:14
#15 0x563f612a3d13 in js::GetObjectElementOperation(JSContext*, JSOp, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) js/src/vm/Interpreter-inl.h:394:10
#16 0x563f6128a14f in js::GetElementOperationWithStackIndex(JSContext*, JS::Handle<JS::Value>, int, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) js/src/vm/Interpreter-inl.h:491:10
#17 0x563f6128a14f in js::Interpret(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:3116:12
#18 0x563f61273000 in js::RunScript(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:498:13
#19 0x563f612783b1 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, js::AbstractFramePtr, JS::MutableHandle<JS::Value>) js/src/vm/Interpreter.cpp:889:13
#20 0x563f61278bbc in js::Execute(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::MutableHandle<JS::Value>) js/src/vm/Interpreter.cpp:922:10
#21 0x563f614c7ab9 in ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>) js/src/vm/CompilationAndEvaluation.cpp:496:10
#22 0x563f614c7d37 in JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>) js/src/vm/CompilationAndEvaluation.cpp:520:10
#23 0x563f611cfb3e in RunFile(JSContext*, char const*, _IO_FILE*, CompileUtf8, bool, bool) js/src/shell/js.cpp:1324:10
#24 0x563f611cebe5 in Process(JSContext*, char const*, bool, FileKind) js/src/shell/js.cpp
#25 0x563f61187ef9 in ProcessArgs(JSContext*, js::cli::OptionParser*) js/src/shell/js.cpp:11861:10
#26 0x563f61187ef9 in Shell(JSContext*, js::cli::OptionParser*) js/src/shell/js.cpp:12128:12
#27 0x563f6117ed5d in main js/src/shell/js.cpp:12690:12
#28 0x7f59f8ec6d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#29 0x7f59f8ec6e3f in __libc_start_main csu/../csu/libc-start.c:392:3
#30 0x563f61148278 in _start (reproducebuild/dist/bin/js+0x1bd3278) (BuildId: 5d99258d19828e08dce328574edaec05)

Comment 1

[Jan de Mooij [:jandem]]

Reduced:

var m = parseModule("{}", "", "json");
m.evaluationError;

Comment 2

[Iain Ireland [:iain]]

It looks to me like this is probably not security-sensitive, but I don't know this module code very well (or really at all).

If I understand correctly, the module object that is returned from parseModule is only exposed to shell code for testing, and is not accessible in the browser. evaluationError is defined as a getter on that object, even for a JSON module (which I think does not match the spec). Inside the getter we try to access the cyclic module fields for the module. At one point, those would only be missing if we observed the module mid-initialization as part of a GC. But that seems to be false with synthetic JSON modules.

So the easy answer would be to rewrite this getter (and any other similar getters on ModuleObject) to return undefined for synthetic modules. It might be better if we didn't provide those getters at all (although it's probably less important since these aren't exposed).

I'll leave the final determination for somebody who actually knows the module code.

Comment 3

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Bug 1928654: Check hasCyclicModuleFields before calling getters in Shell Object wrappers.

Comment 4

[Jon Coppeard (:jonco)]

Not security sensitive as these objects are only exposed in the shell.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a5a7cd0a141a
Check hasCyclicModuleFields before calling getters in Shell Object wrappers. r=jonco

Comment 6

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/a5a7cd0a141a