New issue
Advanced search Search tips

Issue 874055 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Sep 7
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug

Blocked on:
issue 876608



Sign in to add a comment

Null-dereference READ in v8::Context::Enter

Project Member Reported by ClusterFuzz, Aug 14

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=5916211834257408

Fuzzer: lcamtuf_cross_fuzz
Job Type: linux_cfi_chrome
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  v8::Context::Enter
  blink::V8FileSystemCallback::handleEvent
  blink::V8FileSystemCallback::InvokeAndReportException
  
Sanitizer: cfi (CFI)

Regressed: https://clusterfuzz.com/revisions?job=linux_cfi_chrome&range=582157:582184

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5916211834257408

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Project Member

Comment 1 by ClusterFuzz, Aug 14

Components: Blink>Storage>FileSystem
Labels: Test-Predator-Auto-Components
Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.
Components: Blink>JavaScript
Cc: flackr@chromium.org jbroman@chromium.org yukishiino@chromium.org
Owner: smcgruer@chromium.org
Status: Assigned (was: Untriaged)
In the repro case the "keyframes_value" here:

https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/animation/effect_input.cc?q=third_party/blink/renderer/core/animation/effect_input.cc&sq=package:chromium&g=0&l=698

is a "false" boolean value which is not v8::Object.

Components: -Blink>Storage>FileSystem -Blink>JavaScript Blink>Animation
Labels: Test-Predator-Wrong-Components
Status: Started (was: Assigned)
Cc: -yukishiino@chromium.org smcgruer@chromium.org
Owner: yukishiino@chromium.org
Status: Assigned (was: Started)
The calls to EffectInput::ParseKeyframesArgument can come from a few places:

KeyframeEffect::setKeyframes (https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/animation/keyframe_effect.cc?l=176&rcl=89ee28e188c7d2424f6de340db8333a5f273c1f9)

ElementAnimation::animate (https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/animation/element_animation.cc?l=34&rcl=89ee28e188c7d2424f6de340db8333a5f273c1f9 and https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/animation/element_animation.cc?l=55&rcl=89ee28e188c7d2424f6de340db8333a5f273c1f9)

KeyframeEffect::Create (https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/animation/keyframe_effect.cc?sq=package:chromium&g=0&l=82&rcl=89ee28e188c7d2424f6de340db8333a5f273c1f9 and https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/animation/keyframe_effect.cc?sq=package:chromium&g=0&l=100&rcl=89ee28e188c7d2424f6de340db8333a5f273c1f9)

All three of these go via the bindings layer, and all are specified as 'object?'. From talking to jbroman@, I *think* the bindings layer shouldn't have allowed the 'false' boolean value to be passed in. Yuki can you take a look and lmk if you agree (that bindings should be rejecting these non-object values)?


Components: Blink>Bindings
Cc: yukishiino@chromium.org
Owner: peria@chromium.org
Oops, the bindings layer should check if the argument is either of v8::Object or v8::Null.  You're right.

Blockedon: 876608
Verification of IDLObject was rolled in ( issue 876608 ), but this test still fails with the same stack dump.
Cc: peria@chromium.org
Owner: smcgruer@chromium.org
Status: Started (was: Assigned)
Thanks for fixing  issue 876608 !

I'll have a look into what's going on now with the test. Annoyingly I can't seem to get it to work with the debug flag; I get a bunch of CFI compilation errors.
Cc: nhiroki@chromium.org
Components: -Blink>Bindings -Blink>Animation Blink>Storage>FileSystem
Owner: ----
Status: Untriaged (was: Started)
Added the following to effect_input.cc:

diff --git a/third_party/blink/renderer/core/animation/effect_input.cc b/third_party/blink/renderer/core/animation/effect_input.cc
index 8601e570b957..5fddfc7da9a7 100644
--- a/third_party/blink/renderer/core/animation/effect_input.cc
+++ b/third_party/blink/renderer/core/animation/effect_input.cc
@@ -695,6 +695,8 @@ StringKeyframeVector EffectInput::ParseKeyframesArgument(
   v8::Local<v8::Value> keyframes_value = keyframes.V8Value();
   if (keyframes_value->IsNullOrUndefined())
     return {};
+  if (!keyframes_value->IsObject())
+    LOG(FATAL) << "Keyframes value is not an object";
   v8::Local<v8::Object> keyframes_obj = keyframes_value.As<v8::Object>();
 
   // 3. Let method be the result of GetMethod(object, @@iterator).

Re-running the reproduction does not crash here, so the keyframes_value in the test is still an object. I'm inclined to think the animations link is a red herring, given the full stacktrace of the actual crash:

#0 0x56328f3945dc base::debug::StackTrace::StackTrace()
#1 0x56328f3942a1 base::debug::(anonymous namespace)::StackDumpSignalHandler(int, siginfo_t*, void*)$53ec53237a3cc310c6b17019935cd5f1
#2 0x7fd2a787b0c0 <unknown>
#3 0x56328e30f2c6 v8::Context::Enter()
#4 0x563293467026 blink::V8FileSystemCallback::handleEvent()
#5 0x563293467234 blink::V8FileSystemCallback::InvokeAndReportException()
#6 0x5632934610f8 void blink::FileSystemCallbacksBase::InvokeOrScheduleCallback<void (blink::FileSystemCallbacks::OnDidOpenFileSystemCallback::*)(blink::DOMFileSystem*), blink::FileSystemCallbacks::OnDidOpenFileSystemCallback*, blink::DOMFileSystem*>(void (blink::FileSystemCallbacks::OnDidOpenFileSystemCallback::*&&)(blink::DOMFileSystem*), blink::FileSystemCallbacks::OnDidOpenFileSystemCallback*&&, blink::DOMFileSystem*&&)
#7 0x563293461029 blink::FileSystemCallbacks::DidOpenFileSystem()
#8 0x563293457467 blink::WebFileSystemCallbacks::DidOpenFileSystem()
#9 0x563293a0c127 content::(anonymous namespace)::DidOpenFileSystem(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, GURL const&, blink::WebFileSystemCallbacks*)$031ec8683ee9f33a819ea1d8220cea8e
#10 0x563293a0c210 base::internal::Invoker<base::internal::BindState<void (*)(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, GURL const&, blink::WebFileSystemCallbacks*), std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, GURL>, void (blink::WebFileSystemCallbacks*)>::RunOnce(base::internal::BindStateBase*, blink::WebFileSystemCallbacks*).cfi
#11 0x563293a0c1b5 base::OnceCallback<void (blink::WebFileSystemCallbacks*)>::Run(blink::WebFileSystemCallbacks*) &&
#12 0x563293a0c07e content::(anonymous namespace)::RunCallbacks()
#13 0x563293a0acb2 content::(anonymous namespace)::OpenFileSystemCallbackAdapter(int, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, GURL const&)$031ec8683ee9f33a819ea1d8220cea8e
#14 0x563293a0f1df content::FileSystemDispatcher::DidOpenFileSystem()
#15 0x563293a1414a void base::internal::InvokeHelper<false, void>::MakeItSo<void (content::FileSystemDispatcher::*)(int, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, GURL const&, base::File::Error), content::FileSystemDispatcher*, int, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, GURL const&, base::File::Error>(void (content::FileSystemDispatcher::*&&)(int, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, GURL const&, base::File::Error), content::FileSystemDispatcher*&&, int&&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, GURL const&, base::File::Error&&)
#16 0x563293a1410a void base::internal::Invoker<base::internal::BindState<void (content::FileSystemDispatcher::*)(int, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, GURL const&, base::File::Error), base::internal::UnretainedWrapper<content::FileSystemDispatcher>, int>, void (std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, GURL const&, base::File::Error)>::RunImpl<void (content::FileSystemDispatcher::*)(int, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, GURL const&, base::File::Error), std::__1::tuple<base::internal::UnretainedWrapper<content::FileSystemDispatcher>, int>, 0ul, 1ul>(void (content::FileSystemDispatcher::*&&)(int, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, GURL const&, base::File::Error), std::__1::tuple<base::internal::UnretainedWrapper<content::FileSystemDispatcher>, int>&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul>, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, GURL const&, base::File::Error&&)
#17 0x563293a14058 base::internal::Invoker<base::internal::BindState<void (content::FileSystemDispatcher::*)(int, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, GURL const&, base::File::Error), base::internal::UnretainedWrapper<content::FileSystemDispatcher>, int>, void (std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, GURL const&, base::File::Error)>::RunOnce(base::internal::BindStateBase*, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, GURL const&, base::File::Error).cfi
#18 0x56328cf2c685 base::OnceCallback<void (std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, GURL const&, base::File::Error)>::Run(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, GURL const&, base::File::Error) &&
#19 0x56328cf2c514 blink::mojom::FileSystemManager_Open_ForwardToCallback::Accept()
#20 0x56328f3f3ffb mojo::InterfaceEndpointClient::HandleValidatedMessage()
#21 0x56328f3fb037 mojo::internal::MultiplexRouter::ProcessIncomingMessage()
#22 0x56328f3fab4e mojo::internal::MultiplexRouter::Accept()
#23 0x56328f3f16fc mojo::Connector::ReadSingleMessage()
#24 0x56328f3f1cb6 mojo::Connector::ReadAllAvailableMessages()
#25 0x56328f3f254c void base::internal::Invoker<base::internal::BindState<void (mojo::Connector::*)(unsigned int), base::internal::UnretainedWrapper<mojo::Connector> >, void (unsigned int)>::RunImpl<void (mojo::Connector::* const&)(unsigned int), std::__1::tuple<base::internal::UnretainedWrapper<mojo::Connector> > const&, 0ul>(void (mojo::Connector::* const&)(unsigned int), std::__1::tuple<base::internal::UnretainedWrapper<mojo::Connector> > const&, std::__1::integer_sequence<unsigned long, 0ul>, unsigned int&&)
#26 0x56328f3f24bf base::internal::Invoker<base::internal::BindState<void (mojo::Connector::*)(unsigned int), base::internal::UnretainedWrapper<mojo::Connector> >, void (unsigned int)>::Run(base::internal::BindStateBase*, unsigned int).cfi
#27 0x56328d048235 base::internal::Invoker<base::internal::BindState<void (*)(base::RepeatingCallback<void (unsigned int)> const&, unsigned int, mojo::HandleSignalsState const&), base::RepeatingCallback<void (unsigned int)> >, void (unsigned int, mojo::HandleSignalsState const&)>::Run(base::internal::BindStateBase*, unsigned int, mojo::HandleSignalsState const&).cfi
#28 0x56328f40f6ee mojo::SimpleWatcher::OnHandleReady()
#29 0x56328ca300f3 base::OnceCallback<void ()>::Run() &&
#30 0x56328f2fd0c3 base::debug::TaskAnnotator::RunTask()
#31 0x56328f34ee53 base::sequence_manager::internal::ThreadControllerImpl::DoWork()
#32 0x56328ca300f3 base::OnceCallback<void ()>::Run() &&
#33 0x56328f2fd0c3 base::debug::TaskAnnotator::RunTask()
#34 0x56328f2fb2c1 base::MessageLoop::RunTask()
#35 0x56328f2fb58c base::MessageLoop::DeferOrRunPendingTask()
#36 0x56328f2fb800 base::MessageLoop::DoWork()
#37 0x56328f2ff02c base::MessagePumpDefault::Run()
#38 0x56328f323908 base::RunLoop::Run()
#39 0x563293fe971a content::RendererMain(content::MainFunctionParams const&).cfi
#40 0x56328ee02710 content::RunZygote()
#41 0x56328ee04057 content::ContentMainRunnerImpl::Run()
#42 0x56328ee5a013 service_manager::Main()
#43 0x56328ee02491 content::ContentMain()
#44 0x56328c6d6d3c base::LazyInstance<>::OnExit()
#45 0x7fd2a08be2b1 __libc_start_main
#46 0x56328c56102a _start


Passing this over to the file system team (cc nhiroki@ at random from the OWNERS file) to see if they can do some investigation.

Owner: mek@chromium.org
+mek@, can you triage this?
mek@ Could you please look into this issue
Cc: mek@chromium.org
Owner: pwnall@chromium.org
pwnall@ can you take a look? Not sure what's going on, but I thought we had other bugs for crashes in this area at some point?
Project Member

Comment 15 by ClusterFuzz, Sep 7

ClusterFuzz has detected this issue as fixed in range 589184:589185.

Detailed report: https://clusterfuzz.com/testcase?key=5916211834257408

Fuzzer: lcamtuf_cross_fuzz
Job Type: linux_cfi_chrome
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  v8::Context::Enter
  blink::V8FileSystemCallback::handleEvent
  blink::V8FileSystemCallback::InvokeAndReportException
  
Sanitizer: cfi (CFI)

Regressed: https://clusterfuzz.com/revisions?job=linux_cfi_chrome&range=582157:582184
Fixed: https://clusterfuzz.com/revisions?job=linux_cfi_chrome&range=589184:589185

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5916211834257408

See https://github.com/google/clusterfuzz-tools for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 16 by ClusterFuzz, Sep 7

Labels: ClusterFuzz-Verified
Status: Verified (was: Untriaged)
ClusterFuzz testcase 5916211834257408 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

Sign in to add a comment