Closed Bug 1827359 Opened 3 years ago Closed 3 years ago

Intermittent Assertion failure: aIndex < mLength, at /builds/worker/workspace/obj-build/dist/include/mozilla/Vector.h:589

Categories

(Core :: JavaScript Engine, defect, P1)

Product:
Core
Component:
JavaScript Engine
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
114 Branch
Tracking Flags:
Tracking Status
firefox-esr102 113+ fixed
firefox112 --- wontfix
firefox113 + fixed
firefox114 + fixed

People

(Reporter: intermittent-bug-filer, Assigned: bthrall)

References

(Blocks 1 open bug, Regressed 1 open bug)

Details

(4 keywords, Whiteboard: [adv-main113+r][adv-ESR102.11+r])

Attachments

(1 file)

Bug 1827359 - Add release assert to CompilationAtomCache::getExistingStringAt() r=jandem
48 bytes, text/x-phabricator-request
RyanVM
: approval-mozilla-beta+
RyanVM
: approval-mozilla-esr102+
tjr
: sec-approval+
Details | Review

Filed by: ncsoregi [at] mozilla.com
Parsed log: https://treeherder.mozilla.org/logviewer?job_id=411927901&repo=mozilla-central
Full log: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/Tmc5cAPBSjGPsfZ1gJvsiw/runs/0/artifacts/public/logs/live_backing.log

[task 2023-04-11T04:48:00.798Z] 04:48:00     INFO - GECKO(1969) | Assertion failure: aIndex < mLength, at /builds/worker/workspace/obj-build/dist/include/mozilla/Vector.h:589
[task 2023-04-11T04:48:00.802Z] 04:48:00     INFO - checking window state
[task 2023-04-11T04:48:00.805Z] 04:48:00     INFO -  Initializing stack-fixing for the first stack frame, this may take a while...
[task 2023-04-11T04:48:24.166Z] 04:48:24     INFO - GECKO(1969) | #01: js::frontend::CompilationAtomCache::getExistingStringAt(JSContext*, js::frontend::TaggedParserAtomIndex) const [js/src/frontend/Stencil.cpp:4597]
[task 2023-04-11T04:48:24.166Z] 04:48:24     INFO - GECKO(1969) | #02: js::frontend::CompilationAtomCache::getExistingAtomAt(JSContext*, js::frontend::TaggedParserAtomIndex) const [js/src/frontend/Stencil.cpp:4633]
[task 2023-04-11T04:48:24.167Z] 04:48:24     INFO - GECKO(1969) | #03: js::InterpretObjLiteralShape<(js::PropertySetKind)0>(JSContext*, js::frontend::CompilationAtomCache const&, mozilla::Span<unsigned char const, (unsigned long)18446744073709551615>, unsigned int) [js/src/frontend/ObjLiteral.cpp:310]
[task 2023-04-11T04:48:24.167Z] 04:48:24     INFO - GECKO(1969) | #04: js::ObjLiteralStencil::create(JSContext*, js::frontend::CompilationAtomCache const&) const [js/src/frontend/ObjLiteral.cpp:384]
[task 2023-04-11T04:48:24.168Z] 04:48:24     INFO - GECKO(1969) | #05: js::frontend::EmitScriptThingsVector(JSContext*, js::frontend::CompilationAtomCache const&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&, mozilla::Span<js::frontend::TaggedScriptThingIndex const, (unsigned long)18446744073709551615>, mozilla::Span<JS::GCCellPtr, (unsigned long)18446744073709551615>) [js/src/frontend/BytecodeSection.cpp:90]
[task 2023-04-11T04:48:24.169Z] 04:48:24     INFO - GECKO(1969) | #06: js::PrivateScriptData::InitFromStencil(JSContext*, JS::Handle<JSScript*>, js::frontend::CompilationAtomCache const&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&, js::frontend::TypedIndex<js::frontend::ScriptStencil>) [js/src/vm/JSScript.cpp:2287]
[task 2023-04-11T04:48:24.169Z] 04:48:24     INFO - GECKO(1969) | #07: JSScript::fullyInitFromStencil(JSContext*, js::frontend::CompilationAtomCache const&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&, JS::Handle<JSScript*>, js::frontend::TypedIndex<js::frontend::ScriptStencil>) [js/src/vm/JSScript.cpp:2411]
[task 2023-04-11T04:48:24.170Z] 04:48:24     INFO - GECKO(1969) | #08: JSScript::fromStencil(JSContext*, js::frontend::CompilationAtomCache&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&, js::frontend::TypedIndex<js::frontend::ScriptStencil>) [js/src/vm/JSScript.cpp:2499]
[task 2023-04-11T04:48:24.170Z] 04:48:24     INFO - GECKO(1969) | #09: js::frontend::CompilationStencil::instantiateStencilAfterPreparation(JSContext*, js::frontend::CompilationInput&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&) [js/src/frontend/Stencil.cpp:2480]
[task 2023-04-11T04:48:24.171Z] 04:48:24     INFO - GECKO(1969) | #10: js::frontend::CompilationStencil::instantiateStencils(JSContext*, js::frontend::CompilationInput&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&) [js/src/frontend/Stencil.cpp:2413]
[task 2023-04-11T04:48:24.171Z] 04:48:24     INFO - GECKO(1969) | #11: js::frontend::InstantiateStencils(JSContext*, js::frontend::CompilationInput&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&) [js/src/frontend/BytecodeCompiler.cpp:452]
[task 2023-04-11T04:48:24.172Z] 04:48:24     INFO - GECKO(1969) | #12: JS::InstantiateGlobalStencil(JSContext*, JS::InstantiateOptions const&, js::frontend::CompilationStencil*, JS::InstantiationStorage*) [js/src/frontend/Stencil.cpp:5284]
[task 2023-04-11T04:48:24.172Z] 04:48:24     INFO - GECKO(1969) | #13: mozJSSubScriptLoader::DoLoadSubScriptWithOptions(nsTSubstring<char16_t> const&, LoadSubScriptOptions&, JSContext*, JS::MutableHandle<JS::Value>) [js/xpconnect/loader/mozJSSubScriptLoader.cpp:473]
[task 2023-04-11T04:48:24.173Z] 04:48:24     INFO - GECKO(1969) | #14: mozJSSubScriptLoader::LoadSubScript(nsTSubstring<char16_t> const&, JS::Handle<JS::Value>, JSContext*, JS::MutableHandle<JS::Value>) [js/xpconnect/loader/mozJSSubScriptLoader.cpp:298]
[task 2023-04-11T04:48:24.173Z] 04:48:24     INFO - GECKO(1969) | #15: NS_InvokeByIndex [/opt/worker/tasks/task_168118762390514/build/application/Firefox NightlyDebug.app/Contents/MacOS/XUL + 0x337ade]
[task 2023-04-11T04:48:24.174Z] 04:48:24     INFO - GECKO(1969) | #16: CallMethodHelper::Call() [js/xpconnect/src/XPCWrappedNative.cpp:1179]
[task 2023-04-11T04:48:24.174Z] 04:48:24     INFO - GECKO(1969) | #17: XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) [js/xpconnect/src/XPCWrappedNative.cpp:1125]
[task 2023-04-11T04:48:24.175Z] 04:48:24     INFO - GECKO(1969) | #18: XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) [js/xpconnect/src/XPCWrappedNativeJSOps.cpp:965]
[task 2023-04-11T04:48:24.175Z] 04:48:24     INFO - GECKO(1969) | #19: CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) [js/src/vm/Interpreter.cpp:486]
[task 2023-04-11T04:48:24.176Z] 04:48:24     INFO - GECKO(1969) | #20: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) [js/src/vm/Interpreter.cpp:580]
[task 2023-04-11T04:48:24.176Z] 04:48:24     INFO - GECKO(1969) | #21: js::Interpret(JSContext*, js::RunState&) [js/src/vm/Interpreter.cpp:3395]
[task 2023-04-11T04:48:24.176Z] 04:48:24     INFO - GECKO(1969) | #22: js::RunScript(JSContext*, js::RunState&) [js/src/vm/Interpreter.cpp:458]
[task 2023-04-11T04:48:24.177Z] 04:48:24     INFO - GECKO(1969) | #23: js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, js::AbstractFramePtr, JS::MutableHandle<JS::Value>) [js/src/vm/Interpreter.cpp:845]
[task 2023-04-11T04:48:24.177Z] 04:48:24     INFO - GECKO(1969) | #24: ExecuteInExtensibleLexicalEnvironment(JSContext*, JS::Handle<JSScript*>, JS::Handle<js::ExtensibleLexicalEnvironmentObject*>) [js/src/builtin/Eval.cpp:398]
[task 2023-04-11T04:48:24.177Z] 04:48:24     INFO - GECKO(1969) | #25: js::ExecuteInFrameScriptEnvironment(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::MutableHandle<JSObject*>) [js/src/builtin/Eval.cpp:431]
[task 2023-04-11T04:48:24.178Z] 04:48:24     INFO - GECKO(1969) | #26: nsMessageManagerScriptExecutor::LoadScriptInternal(JS::Handle<JSObject*>, nsTSubstring<char16_t> const&, bool) [dom/base/nsFrameMessageManager.cpp:1216]
[task 2023-04-11T04:48:24.178Z] 04:48:24     INFO - GECKO(1969) | #27: mozilla::dom::BrowserChild::RecvLoadRemoteScript(nsTSubstring<char16_t> const&, bool const&) [dom/ipc/BrowserChild.cpp:2209]
[task 2023-04-11T04:48:24.178Z] 04:48:24     INFO - GECKO(1969) | #28: mozilla::dom::PBrowserChild::OnMessageReceived(IPC::Message const&) [s3:gecko-generated-sources:d911fcc86a3b69c3a9cc1ede3fa6fba2bc9154330a5800822d41fc185a79ffb1a9a14572470772c22dc392f3c66c6611c0ef47aaffa11555463b7e89618f795d/ipc/ipdl/PBrowserChild.cpp::0]
[task 2023-04-11T04:48:24.179Z] 04:48:24     INFO - GECKO(1969) | #29: mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) [s3:gecko-generated-sources:9388f0536c43a6e393e2844442ecb51ab33ce286a8e9ee5736ec60128cf2a84f90fcd5f2ad3093000b411e4a1d3d1a4cbff9d5030d03fb1ffa8e833d9c4b8e29/ipc/ipdl/PContentChild.cpp::8768]
[task 2023-04-11T04:48:24.179Z] 04:48:24     INFO - TEST-START | browser/base/content/test/tabs/browser_tab_tooltips.js
[task 2023-04-11T04:48:24.179Z] 04:48:24     INFO - GECKO(1969) | #30: mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) [ipc/glue/MessageChannel.cpp:1800]
[task 2023-04-11T04:48:24.180Z] 04:48:24     INFO - GECKO(1969) | #31: mozilla::ipc::MessageChannel::DispatchMessage(mozilla::ipc::ActorLifecycleProxy*, mozilla::UniquePtr<IPC::Message, mozilla::DefaultDelete<IPC::Message> >) [ipc/glue/MessageChannel.cpp:0]
[task 2023-04-11T04:48:24.180Z] 04:48:24     INFO - GECKO(1969) | #32: mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::ActorLifecycleProxy*, mozilla::ipc::MessageChannel::MessageTask&) [ipc/glue/MessageChannel.cpp:1525]
[task 2023-04-11T04:48:24.180Z] 04:48:24     INFO - GECKO(1969) | #33: mozilla::ipc::MessageChannel::MessageTask::Run() [ipc/glue/MessageChannel.cpp:1632]
[task 2023-04-11T04:48:24.180Z] 04:48:24     INFO - GECKO(1969) | #34: mozilla::RunnableTask::Run() [xpcom/threads/TaskController.cpp:554]
[task 2023-04-11T04:48:24.181Z] 04:48:24     INFO - GECKO(1969) | #35: mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) [xpcom/threads/TaskController.cpp:869]
[task 2023-04-11T04:48:24.181Z] 04:48:24     INFO - GECKO(1969) | #36: mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) [xpcom/threads/TaskController.cpp:0]
[task 2023-04-11T04:48:24.181Z] 04:48:24     INFO - GECKO(1969) | #37: mozilla::TaskController::ProcessPendingMTTask(bool) [xpcom/threads/TaskController.cpp:464]
[task 2023-04-11T04:48:24.182Z] 04:48:24     INFO - GECKO(1969) | #38: mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_4>::Run() [xpcom/threads/nsThreadUtils.h:549]
[task 2023-04-11T04:48:24.182Z] 04:48:24     INFO - GECKO(1969) | #39: nsThread::ProcessNextEvent(bool, bool*) [xpcom/threads/nsThread.cpp:1243]
[task 2023-04-11T04:48:24.182Z] 04:48:24     INFO - GECKO(1969) | #40: NS_ProcessNextEvent(nsIThread*, bool) [xpcom/threads/nsThreadUtils.cpp:479]
[task 2023-04-11T04:48:24.183Z] 04:48:24     INFO - GECKO(1969) | #41: mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) [ipc/glue/MessagePump.cpp:86]
[task 2023-04-11T04:48:24.183Z] 04:48:24     INFO - GECKO(1969) | #42: MessageLoop::Run() [ipc/chromium/src/base/message_loop.cc:345]
[task 2023-04-11T04:48:24.183Z] 04:48:24     INFO - GECKO(1969) | #43: nsBaseAppShell::Run() [widget/nsBaseAppShell.cpp:150]
[task 2023-04-11T04:48:24.183Z] 04:48:24     INFO - GECKO(1969) | #44: nsAppShell::Run() [widget/cocoa/nsAppShell.mm:806]
[task 2023-04-11T04:48:24.184Z] 04:48:24     INFO - GECKO(1969) | #45: XRE_RunAppShell() [toolkit/xre/nsEmbedFunctions.cpp:738]
[task 2023-04-11T04:48:24.184Z] 04:48:24     INFO - GECKO(1969) | [Child 2614: Main Thread]: I/DocShellAndDOMWindowLeak ++DOCSHELL 12994a000 == 1 [pid = 2614] [id = 12]
[task 2023-04-11T04:48:24.184Z] 04:48:24     INFO - GECKO(1969) | [Child 2614: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 1 (11ed145e0) [pid = 2614] [serial = 36] [outer = 0]
[task 2023-04-11T04:48:24.185Z] 04:48:24     INFO - GECKO(1969) | #46: mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) [ipc/glue/MessagePump.cpp:235]
[task 2023-04-11T04:48:24.185Z] 04:48:24     INFO - GECKO(1969) | [Child 2614: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 2 (12994a800) [pid = 2614] [serial = 37] [outer = 11ed145e0]
[task 2023-04-11T04:48:24.185Z] 04:48:24     INFO - GECKO(1969) | #47: MessageLoop::Run() [ipc/chromium/src/base/message_loop.cc:345]
[task 2023-04-11T04:48:24.186Z] 04:48:24     INFO - GECKO(1969) | #48: XRE_InitChildProcess(int, char**, XREChildData const*) [toolkit/xre/nsEmbedFunctions.cpp:0]
[task 2023-04-11T04:48:24.186Z] 04:48:24     INFO - GECKO(1969) | #49: main [ipc/app/MozillaRuntimeMain.cpp:90]
Component: General → JavaScript Engine
Group: javascript-core-security
Keywords: csectype-bounds

I'm not sure what is going wrong here, but it looks like a buffer overflow. I looked at the minidump in the log and the other threads all looked like they were waiting around.

Keywords: sec-high

The severity field for this bug is set to S4. However, the bug is flagged with the sec-high keyword.
:willyelm, could you consider increasing the severity of this security bug?

For more information, please visit auto_nag documentation.

Flags: needinfo?(wmedina)

Marking this S2/P1 since this is a Sec-High and someone needs to investigate. After some initial discussion this could possibly be unactionable bug with the information we have to date.

Severity: S4 → S2
Flags: needinfo?(wmedina)
Priority: P5 → P1

Bryan, would you be willing to do a quick initial investigation of this bug. You can work with Ted/Jan on how to approach that.

Flags: needinfo?(bthrall)
Assignee: nobody → bthrall
Flags: needinfo?(bthrall)

One way to paper this over would be to add an explicit bounds check. Hopefully the performance cost is small enough. (Bounds checks were added to all nsTArray indexing without any real impact FWIW.)

I don't see any crashes in crash-stats.mozilla.org that match this stack trace, and I don't see anything suspicious in the relevant code.

The most likely cause is corrupted Stencil data read from the disk cache, but we can't rule out other causes (like use after free).

I like :mccr8's suggestion to add an explicit bounds check; we should explore how much that would affect performance.

At this point, I would say we don't have enough information to act other than exploring an explicit bounds check.

Assignee: bthrall → nobody

Adding a bounds check as MOZ_RELEASE_ASSERT for just this call site will probably be fine. We could also go with MOZ_DIAGNOSTIC_ASSERT, this way it's disabled on release but we'd still get information from Nightly/early-beta crash reports.

To expand on comment 6:

The crash is happening in a CompilationAtomCache, which is part of the CompilationInput, but the CompilationInput is created on the stack and the CompilationAtomCache's atom array is allocated here so a logic error in the atom indexing or use after free looks unlikely.

:tcampbell and I searched crash-stats for 'getExistingStringAt' and found some reports in release that might be related to this assertion failure. Some of the reports, and a report in nightly, are from shutdownhang or ShutDownKill, so that might be another symptom. (These results are different from what I reported in comment 6 because I was not searching crash-stats properly before now).

Also, :tcampbell noted that mozJSSubScriptLoader in the stack trace indicates that this crash is happening while processing Firefox internal code, so it is unlikely that the problem is being triggered by some exotic web code.

I will go ahead and add the MOZ_RELEASE_ASSERT bounds check, and we can proceed from there.

Assignee: nobody → bthrall

I ran benchmark performance on Windows and there doesn't look like any performance impact from adding the bounds check.

Comment on attachment 9329503 [details]
Bug 1827359 - Add release assert to CompilationAtomCache::getExistingStringAt() r=jandem

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: Not easily. Source of crash unknown, possibly related to memory corruption. The patch adds a diagnostic crash and does not identify an exploit.
  • 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
  • If not all supported branches, which bug introduced the flaw?: None
  • Do you have backports for the affected branches?: No
  • If not, how different, hard to create, and risky will they be?: The patch applies to ESR, release, and beta directly.
  • How likely is this patch to cause regressions; how much testing does it need?: It is very unlikely. We are not changing the behavior of the software except that we are adding a safe release assertion where previously there was unknown behavior.

Test suite has great coverage of the changed code.

  • Is Android affected?: Yes
Attachment #9329503 - Flags: sec-approval?

Given that this patch just adds a release assertion, is it worth uplifting?

If it means we're crashing safely instead of getting into an exploitable situation, that seems like an improvement worth uplifting IMO.

Comment on attachment 9329503 [details]
Bug 1827359 - Add release assert to CompilationAtomCache::getExistingStringAt() r=jandem

Approved to land and uplift

Attachment #9329503 - Flags: sec-approval? → sec-approval+

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

Group: javascript-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 3 years ago
status-firefox114: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → 114 Branch

The patch landed in nightly and beta is affected.
:bthrall, is this bug important enough to require an uplift?

  • If yes, please nominate the patch for beta approval.
  • If no, please set status-firefox113 to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(bthrall)

Comment on attachment 9329503 [details]
Bug 1827359 - Add release assert to CompilationAtomCache::getExistingStringAt() r=jandem

Beta/Release Uplift Approval Request

  • User impact if declined: The user's system is potentially exploitable due to a buffer overflow.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce: We don't have a known list of steps to reproduce the buffer overflow.
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): It does not change the logical behavior of the application. It will crash the application in a known state before the overflow can happen.

The change has not triggered any increase in crashes in nightly over the past two days.

  • String changes made/needed:
  • Is Android affected?: Yes
Flags: needinfo?(bthrall)
Attachment #9329503 - Flags: approval-mozilla-beta?

Comment on attachment 9329503 [details]
Bug 1827359 - Add release assert to CompilationAtomCache::getExistingStringAt() r=jandem

Approved for 113.0b8 and 102.11esr.

Attachment #9329503 - Flags: approval-mozilla-esr102+
Attachment #9329503 - Flags: approval-mozilla-beta?
Attachment #9329503 - Flags: approval-mozilla-beta+
Whiteboard: [adv-main113+r]
Whiteboard: [adv-main113+r] → [adv-main113+r][adv-ESR102.11+]
Whiteboard: [adv-main113+r][adv-ESR102.11+] → [adv-main113+r][adv-ESR102.11+r]
Flags: qe-verify-
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: