Bug 81741

This is stackTrace in chromium port. #0 0x00007f7ba6e5959d in WebCore::Node::document (this=0x0) at third_party/WebKit/Source #1 0x00007f7ba774c0d2 in WebCore::VisibleSelection::firstRange (this=0x7f7ba435ec48) at third_party/WebKit/Source/WebCore/editing/VisibleSelection.cpp:132 #2 0x00007f7ba6e93583 in WebKit::WebViewImpl::caretOrSelectionRange (this=0x7f7ba436a800 length=0x7fffecf67d68) at third_party/WebKit/Source/WebKit/chromium/src/WebViewImpl.c #3 0x00007f7ba8bdae1e in RenderViewImpl::SyncSelectionIfRequired (this=0x7f7ba42d5400) a #4 0x00007f7ba8bd1088 in RenderViewImpl::didChangeSelection (this=0x7f7ba42d5400, is_emp at content/renderer/render_view_impl.cc:1697 #5 0x00007f7ba6eb854d in WebKit::EditorClientImpl::respondToChangedSelection (this=0x7f7 at third_party/WebKit/Source/WebKit/chromium/src/EditorClientImpl.cpp:266 #6 0x00007f7ba76fa910 in WebCore::Editor::respondToChangedSelection (this=0x7f7ba435eb60 at third_party/WebKit/Source/WebCore/editing/Editor.cpp:482 #7 0x00007f7ba7707f52 in WebCore::Editor::respondToChangedSelection (this=0x7f7ba435eb60 at third_party/WebKit/Source/WebCore/editing/Editor.cpp:2938 #8 0x00007f7ba7715295 in WebCore::FrameSelection::setSelection (this=0x7f7ba435ec20, new align=WebCore::FrameSelection::AlignCursorOnScrollIfNeeded, granularity=WebCore::Char at third_party/WebKit/Source/WebCore/editing/FrameSelection.cpp:298 #9 0x00007f7ba6e6590d in WebCore::FrameSelection::setSelection (this=0x7f7ba435ec20, sel granularity=WebCore::CharacterGranularity) at third_party/WebKit/Source/WebCore/editi #10 0x00007f7ba7714eae in WebCore::FrameSelection::setNonDirectionalSelectionIfNeeded (th granularity=WebCore::CharacterGranularity, endpointsAdjustmentMode=WebCore::FrameSele at third_party/WebKit/Source/WebCore/editing/FrameSelection.cpp:241 #11 0x00007f7ba78a371c in WebCore::EventHandler::updateSelectionForMouseDownDispatchingSe targetNode=0x7f7ba436d380, newSelection=..., granularity=WebCore::CharacterGranularit at third_party/WebKit/Source/WebCore/page/EventHandler.cpp:409 #12 0x00007f7ba78a42d0 in WebCore::EventHandler::handleMousePressEventSingleClick (this=0 at third_party/WebKit/Source/WebCore/page/EventHandler.cpp:544 #13 0x00007f7ba78a46b8 in WebCore::EventHandler::handleMousePressEvent (this=0x7f7ba435ed at third_party/WebKit/Source/WebCore/page/EventHandler.cpp:624 #14 0x00007f7ba78a7ca6 in WebCore::EventHandler::handleMousePressEvent (this=0x7f7ba435ed at third_party/WebKit/Source/WebCore/page/EventHandler.cpp:1617 #15 0x00007f7ba6e8e3fd in WebKit::WebViewImpl::mouseDown (this=0x7f7ba436a800, event=...) at third_party/WebKit/Source/WebKit/chromium/src/WebViewImpl.cpp:499 #16 0x00007f7ba6e921f9 in WebKit::WebViewImpl::handleInputEvent (this=0x7f7ba436a800, inp at third_party/WebKit/Source/WebKit/chromium/src/WebViewImpl.cpp:1596 #17 0x00007f7ba8bf2952 in RenderWidget::OnHandleInputEvent (this=0x7f7ba42d5400, message= #18 0x00007f7ba8bf8dbb in IPC::Message::Dispatch<RenderWidget, RenderWidget> (msg=0x7f7b9 sender=0x7f7ba42d5400, func= (void (RenderWidget::*)(RenderWidget * const, const IPC::Message &)) 0x7f7ba8bf2674 < const&)>) at ./ipc/ipc_message.h:154 #19 0x00007f7ba8bf0a83 in RenderWidget::OnMessageReceived (this=0x7f7ba42d5400, message=. #20 0x00007f7ba8bccb98 in RenderViewImpl::OnMessageReceived (this=0x7f7ba42d5400, message at content/renderer/render_view_impl.cc:838 #21 0x00007f7ba6b06b9c in MessageRouter::RouteMessage (this=0x7f7ba42868f0, msg=...) at c #22 0x00007f7ba6b06b3e in MessageRouter::OnMessageReceived (this=0x7f7ba42868f0, msg=...) #23 0x00007f7ba6a08691 in ChildThread::OnMessageReceived (this=0x7f7ba42868c8, msg=...) a #24 0x00007f7ba6054544 in IPC::ChannelProxy::Context::OnDispatchMessage (this=0x7f7ba4287 at ipc/ipc_channel_proxy.cc:268 #25 0x00007f7ba6057729 in base::internal::RunnableAdapter<void (IPC::ChannelProxy::Contex this=0x7fffecf6a710, object=0x7f7ba4287700, a1=...) at ./base/bind_internal.h:188 #26 0x00007f7ba60572ef in base::internal::InvokeHelper<false, void, base::internal::Runna )(IPC::Message const&)>, void (IPC::ChannelProxy::Context* const&, IPC::Message const&)>: (IPC::ChannelProxy::Context::*)(IPC::Message const&)>, IPC::ChannelProxy::Context* const a1=@0x7f7b96893f20, a2=...) at ./base/bind_internal.h:896 #27 0x00007f7ba6056cae in base::internal::Invoker<2, base::internal::BindState<base::inte :Context::*)(IPC::Message const&)>, void (IPC::ChannelProxy::Context*, IPC::Message const ssage)>, void (IPC::ChannelProxy::Context*, IPC::Message const&)>::Run(base::internal::Bi at ./base/bind_internal.h:1254 #28 0x00007f7ba56f96c3 in base::Callback<void ()>::Run() const (this=0x7fffecf6aa68) at . #29 0x00007f7ba5f9f3b0 in MessageLoop::RunTask (this=0x7fffecf6b370, pending_task=...) at #30 0x00007f7ba5f9f4c7 in MessageLoop::DeferOrRunPendingTask (this=0x7fffecf6b370, pendin #31 0x00007f7ba5f9fce9 in MessageLoop::DoWork (this=0x7fffecf6b370) at base/message_loop. #32 0x00007f7ba5fa79bc in base::MessagePumpDefault::Run (this=0x7f7ba42af180, delegate=0x at base/message_pump_default.cc:28 #33 0x00007f7ba5f9f077 in MessageLoop::RunInternal (this=0x7fffecf6b370) at base/message_ #34 0x00007f7ba5f9ef2a in MessageLoop::RunHandler (this=0x7fffecf6b370) at base/message_l #35 0x00007f7ba5f9e85f in MessageLoop::Run (this=0x7fffecf6b370) at base/message_loop.cc: #36 0x00007f7ba8c072ee in RendererMain (parameters=...) at content/renderer/renderer_main #37 0x00007f7ba5f066d1 in (anonymous namespace)::RunZygote (main_function_params=..., del at content/app/content_main_runner.cc:245 #38 0x00007f7ba5f068f6 in (anonymous namespace)::RunNamedProcessTypeMain (process_type="z delegate=0x7fffecf6bdd0) at content/app/content_main_runner.cc:290 #39 0x00007f7ba5f0711c in (anonymous namespace)::ContentMainRunnerImpl::Run (this=0x7f7ba at content/app/content_main_runner.cc:511 #40 0x00007f7ba5f06031 in content::ContentMain (argc=3, argv=0x7fffecf6bf38, delegate=0x7 at content/app/content_main.cc:35 #41 0x00007f7ba514136d in ChromeMain (argc=3, argv=0x7fffecf6bf38) at chrome/app/chrome_m #42 0x00007f7ba514132c in main (argc=3, argv=0x7fffecf6bf38) at chrome/app/chrome_exe_mai

Created attachment 132988 [details] Patch

I'm not sure VisibleSelection can return null if anchorNode is null... I think rniwa can review this...

Why is this a security bug if it's a clean null crash?

Comment on attachment 132988 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=132988&action=review > Source/WebCore/editing/VisibleSelection.cpp:133 > + if (!start.anchorNode() || !end.anchorNode()) You should check start.isNull() and end.isNull() instead. > ManualTests/meter-in-shadow.html:2 > + Why do we need this blank line? > ManualTests/meter-in-shadow.html:6 > +<head> > + <title>Hoge</title> > +</head> Do we really need this head & title?

(In reply to comment #4) > Why is this a security bug if it's a clean null crash? Sorry, I didn't have a good policy to mark as security bug... Chromium bugs were aften marked as security bug these days, so I used the same strategy...

Created attachment 132992 [details] Patch

(In reply to comment #5) > (From update of attachment 132988 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=132988&action=review > > > Source/WebCore/editing/VisibleSelection.cpp:133 > > + if (!start.anchorNode() || !end.anchorNode()) > > You should check start.isNull() and end.isNull() instead. Done. > > > ManualTests/meter-in-shadow.html:2 > > + > > Why do we need this blank line? Removed. > > > ManualTests/meter-in-shadow.html:6 > > +<head> > > + <title>Hoge</title> > > +</head> > > Do we really need this head & title? No. Removed.

This is not a security bug.

Comment on attachment 132992 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=132992&action=review > ManualTests/meter-in-shadow.html:6 > +var sr = new WebKitShadowRoot(container); Please don't use abbreviations like sr. Also, can't you use event sender here?

(In reply to comment #10) > (From update of attachment 132992 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=132992&action=review > > > ManualTests/meter-in-shadow.html:6 > > +var sr = new WebKitShadowRoot(container); > > Please don't use abbreviations like sr. Also, can't you use event sender here? Oh. I completely forgot we have eventSender... I'll try it before landing this.

Created attachment 132999 [details] Patch

(In reply to comment #10) > (From update of attachment 132992 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=132992&action=review > > > ManualTests/meter-in-shadow.html:6 > > +var sr = new WebKitShadowRoot(container); > > Please don't use abbreviations like sr. Also, can't you use event sender here? Fixed for the abbreviation. I've tried eventSender, but I couldn't reproduce the crash, maybe because chromium try to create selection by right click, but DRT won't. I'm not sure though...

<rdar://problem/11091895>

I found that doubleclick also causes a crash and my previous patch didn't cover it. I'll update the patch soon.

Hmm... in the current trunk this crash didn't happen as far as I tried on Linux and Mac.