New issue
Advanced search Search tips

Issue 49596 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Jul 2010
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security
M-5

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment

Security issue in SVGUseElement::buildShadowTree

Reported by woo...@gmail.com, Jul 20 2010

Issue description

Hi, these two I sent them to ZDI severval months ago , but ZDI refused, so I sent them to google. I tested , it works on newest chrome stable version.

wushi



 
45.rar
3.3 KB Download
47.rar
2.7 KB Download
Labels: -Pri-0 -Area-Undefined Pri-1 Area-WebKit
Status: Available
Summary: Security issue in SVGUseElement::buildShadowTree
Lets use it to track 47.rar attachments. Other attachment is handled in 49628. Need to do more analysis on this.

Breaks at
    shadowRoot->appendChild(newChild.release(), ec);
    wheree newChild is NULL.
 	00000002()	
>WebCore::SVGUseElement::buildShadowTree(WebCore::SVGShadowTreeRootElement * shadowRoot=0x088ab780, WebCore::SVGElement * target=0x0886fe00, WebCore::SVGElementInstance * targetInstance=0x0b0b7f80)  Line 776 + 0x2b bytes	C++
 WebCore::SVGUseElement::buildShadowAndInstanceTree(WebCore::SVGShadowTreeRootElement * shadowRoot=0x088ab780)  Line 538	C++
 WebCore::RenderSVGShadowTreeRootContainer::updateFromElement()  Line 80	C++
 WebCore::updateFromElementCallback(WebCore::Node * node=0x04edaa00)  Line 601 + 0x12 bytes	C++
 WebCore::ContainerNode::dispatchPostAttachCallbacks()  Line 630 + 0x9 bytes	C++
 WebCore::ContainerNode::resumePostAttachCallbacks()  Line 598	C++
 WebCore::Element::attach()  Line 828	C++
 WebCore::ContainerNode::insertBefore(WTF::PassRefPtr<WebCore::Node> newChild={m_document=0x0b7cb000 m_previous=0x00000000 m_next=0x0b3e2c80 ...}, WebCore::Node * refChild=0x0b3e2c80, int & ec=0, bool shouldLazyAttach=false)  Line 167 + 0x12 bytes	C++
 WebCore::XMLDocumentParser::insertErrorMessageBlock()  Line 314	C++
 WebCore::XMLDocumentParser::end()  Line 224	C++
 WebCore::XMLDocumentParser::finish()  Line 240	C++
 WebCore::Document::finishParsing()  Line 2055 + 0x20 bytes	C++
 WebCore::DocumentWriter::endIfNotLoadingMainResource()  Line 222	C++
 WebCore::DocumentWriter::end()  Line 207	C++
 WebCore::DocumentLoader::finishedLoading()  Line 270	C++
 WebCore::FrameLoader::finishedLoading()  Line 2223	C++
 WebCore::MainResourceLoader::didFinishLoading()  Line 435	C++
 WebCore::ResourceLoader::didFinishLoading(WebCore::ResourceHandle * __formal=0x082ed030)  Line 443 + 0xf bytes	C++
 WebCore::ResourceHandleInternal::didFinishLoading(WebKit::WebURLLoader * __formal=0x082f6718)  Line 191 + 0x25 bytes	C++
 webkit_glue::WebURLLoaderImpl::Context::OnCompletedRequest(const URLRequestStatus & status={...}, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & security_info="")  Line 604 + 0x1e bytes	C++
 ResourceDispatcher::OnRequestComplete(int request_id=34, const URLRequestStatus & status={...}, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & security_info="")  Line 467 + 0x17 bytes	C++
 DispatchToMethod<ResourceDispatcher,void (__thiscall ResourceDispatcher::*)(int,URLRequestStatus const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &),int,URLRequestStatus,std::basic_string<char,std::char_traits<char>,std::allocator<char> > >(ResourceDispatcher * obj=0x00bb5320, void (int, const URLRequestStatus &, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > &)* method=0x54fd4340, const Tuple3<int,URLRequestStatus,std::basic_string<char,std::char_traits<char>,std::allocator<char> > > & arg={...})  Line 435 + 0x1c bytes	C++
 IPC::MessageWithTuple<Tuple3<int,URLRequestStatus,std::basic_string<char,std::char_traits<char>,std::allocator<char> > > >::Dispatch<ResourceDispatcher,void (__thiscall ResourceDispatcher::*)(int,URLRequestStatus const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &)>(const IPC::Message * msg=class=16, index=54, ResourceDispatcher * obj=0x00bb5320, void (int, const URLRequestStatus &, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > &)* func=0x54fd4340)  Line 1043 + 0x11 bytes	C++
 ResourceDispatcher::DispatchMessageW(const IPC::Message & message=class=16, index=54)  Line 536 + 0x12 bytes	C++
 ResourceDispatcher::OnMessageReceived(const IPC::Message & message=class=16, index=54)  Line 303	C++
 ChildThread::OnMessageReceived(const IPC::Message & msg=class=16, index=54)  Line 124 + 0x19 bytes	C++
 IPC::ChannelProxy::Context::OnDispatchMessage(const IPC::Message & message=class=16, index=54)  Line 206 + 0x19 bytes	C++
 DispatchToMethod<IPC::ChannelProxy::Context,void (__thiscall IPC::ChannelProxy::Context::*)(IPC::Message const &),IPC::Message>(IPC::ChannelProxy::Context * obj=0x00c5d900, void (const IPC::Message &)* method=0x53eddae0, const Tuple1<IPC::Message> & arg={...})  Line 422 + 0xf bytes	C++
 RunnableMethod<IPC::ChannelProxy::Context,void (__thiscall IPC::ChannelProxy::Context::*)(IPC::Message const &),Tuple1<IPC::Message> >::Run()  Line 326 + 0x1e bytes	C++
 MessageLoop::RunTask(Task * task=0x0b36e980)  Line 409 + 0xf bytes	C++
 MessageLoop::DeferOrRunPendingTask(const MessageLoop::PendingTask & pending_task={...})  Line 421	C++
 MessageLoop::DoWork()  Line 525 + 0xc bytes	C++
 base::MessagePumpForUI::DoRunLoop()  Line 203 + 0x1d bytes	C++
 base::MessagePumpWin::RunWithDispatcher(base::MessagePump::Delegate * delegate=0x0521faa0, base::MessagePumpWin::Dispatcher * dispatcher=0x00000000)  Line 52 + 0xf bytes	C++
 base::MessagePumpWin::Run(base::MessagePump::Delegate * delegate=0x0521faa0)  Line 79 + 0x1c bytes	C++
 MessageLoop::RunInternal()  Line 257 + 0x2a bytes	C++
 MessageLoop::RunHandler()  Line 230	C++
 MessageLoop::Run()  Line 208	C++
 base::Thread::Run(MessageLoop * message_loop=0x0521faa0)  Line 137	C++
 base::Thread::ThreadMain()  Line 160 + 0x16 bytes	C++
 `anonymous namespace'::ThreadFunc(void * closure=0x00c6f780)  Line 26 + 0xf bytes	C++
 	kernel32.dll!@BaseThreadInitThunk@12()  + 0xe bytes	
 	ntdll.dll!___RtlUserThreadStart@8()  + 0x23 bytes	
 	ntdll.dll!__RtlUserThreadStart@8()  + 0x1b bytes	

1.xhtml
5.3 KB View Download

Comment 2 by jsc...@chromium.org, Jul 20 2010

Here's a reduction. It's another SVG USE element bug. The crash is an exec NULL, so it might an arbitrary exec (or harmless). I should know better shortly.

svg_use.xhtml
334 bytes View Download

Comment 3 by jsc...@chromium.org, Jul 20 2010

Labels: SecSeverity-High OS-All Mstone-5
Looks like a stale pointer. I have to dig a bit more, but a style recalculation triggers an early deletion of the SVGGElement exposed by the SVGShadowTreeRootElement.

I've filed upstream with more details: 
https://bugs.webkit.org/show_bug.cgi?id=42659

Labels: reward-500 reward-unpaid
Thank you wushi! We have fixed both of these already. We'll get the fixes out to users in the next release after this week's release.

This qualifies for a $500 reward (as does the other one, which I'll tag separately).

One comment: we're now looking at rewarding $1000 for bugs like this, if the bug report quality is high. If you had taken the big HTML demo file and reduced it to the simplest test case, it would have likely been worth $1000.

Comment 5 by jsc...@chromium.org, Jul 22 2010

Status: WillMerge
Landed as: http://trac.webkit.org/changeset/63865

Comment 6 by jsc...@chromium.org, Jul 22 2010

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
merged to 472. forgot the bug no, so adding the commit info..

Merge 63865 - 2010-07-21 Justin Schuh <jschuh@chromium.org>

Reviewed by Oliver Hunt.

Prevent DeleteButtonController enable state from changing when not editing
https://bugs.webkit.org/show_bug.cgi?id=42659

Test: svg/custom/use-invalid-html.xhtml

* dom/ContainerNode.cpp:
(WebCore::ContainerNode::cloneChildNodes):
2010-07-21 Justin Schuh <jschuh@chromium.org>

Reviewed by Oliver Hunt.

Prevent DeleteButtonController enable state from changing when not editing
https://bugs.webkit.org/show_bug.cgi?id=42659

* svg/custom/use-invalid-html-expected.txt: Added.
* svg/custom/use-invalid-html.xhtml: Added.


TBR=jschuh@chromium.org

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=53682
Status: FixUnreleased
We did merge this to 375:

Merge 63865 - 2010-07-21  Justin Schuh  <jschuh@chromium.org>

        Reviewed by Oliver Hunt.

        Prevent DeleteButtonController enable state from changing when not editing
        https://bugs.webkit.org/show_bug.cgi?id=42659

        Test: svg/custom/use-invalid-html.xhtml

        * dom/ContainerNode.cpp:
        (WebCore::ContainerNode::cloneChildNodes):
2010-07-21  Justin Schuh  <jschuh@chromium.org>

        Reviewed by Oliver Hunt.

        Prevent DeleteButtonController enable state from changing when not editing
        https://bugs.webkit.org/show_bug.cgi?id=42659

        * svg/custom/use-invalid-html-expected.txt: Added.
        * svg/custom/use-invalid-html.xhtml: Added.


Review URL: http://codereview.chromium.org/3007044
Works fine on Mac 5.0.375.127 (Official Build 55887).
Browser doesn't crash.
Works fine with Google Chrome 5.0.375.127 (Official Build 55887) on Windows.

On Linux Ubuntu 9.04 with Google Chrome 5.0.375.127 (Official Build 55887),
renderer crashes with 1.xhtml from comment#1 but doesn't crash with svg_use.xhtml from comment#2

Crash reports haven't got uploaded yet but you can find them @ 
http://crash/reportdetail?reportid=4d0e41ca16494e63
http://crash/reportdetail?reportid=ecea9e26556d6dbc
I don't see the crashed thread in the reports. But it should be fairly easy to repro.
@sunandt, so far I can't get a repro with either of the 1.xhtml files on 5.0.375.127 on Windows. And the linked crash reports are from renderer startup/shutdown hangs, which really doesn't make sense for this bug. 

Specifically which of the files are you referring to, and can you provide anymore detail?

jschuh, this works fine on Windows. Renderer crash can be seen on Linux. Please see comment#12. I'm not sure why the stack trace doesn't show up in crash/.
@sunandt - Please tell me which file you are testing. There are two archives and both have a 1.xhtml file. There's no useful crash data and we need something to work from.

Nevermind. I see it's the file from comment #1, not from the original archive. Sorry for the confusion.
It looks like there's an entirely different bug hidden in all the fuzzer junk. It doesn't show up on Windows or Mac without several minutes of automatic reloading. I'll try to isolate it and open another bug.

After a lot of poking around I found that the original repro can very infrequently trigger  issue 51252 . I'll leave a note in that bug to verify against the repro here after we get a fix in for  issue 51252 .

Do not make this visible until  issue 51252  is patched.

Labels: -reward-unpaid
Payment in the electronic system.

Hey @wooshi, we should not have actually paid out on this one. We require reports to be sent to us and only us. Your first comment mentions offering the bug first to ZDI.

However the mistake is ours so we did of course still pay.
Labels: Type-Security
Labels: SecImpacts-Stable
Batch update.
Labels: -Restrict-View-SecurityNotify
Lifting view restrictions.
Status: Fixed
Project Member

Comment 25 by bugdroid1@chromium.org, Oct 13 2012

Labels: Restrict-AddIssueComment-Commit
Owner: ----
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.
Project Member

Comment 26 by bugdroid1@chromium.org, Mar 10 2013

Labels: -Area-WebKit -SecSeverity-High -Mstone-5 -Type-Security -SecImpacts-Stable Cr-Content M-5 Security-Impact-Stable Type-Bug-Security Security-Severity-High
Project Member

Comment 27 by bugdroid1@chromium.org, Mar 13 2013

Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Project Member

Comment 28 by bugdroid1@chromium.org, Mar 21 2013

Labels: -Security-Severity-High Security_Severity-High
Project Member

Comment 29 by bugdroid1@chromium.org, Mar 21 2013

Labels: -Security-Impact-Stable Security_Impact-Stable
Project Member

Comment 30 by bugdroid1@chromium.org, Apr 6 2013

Labels: -Cr-Content Cr-Blink
Project Member

Comment 31 by sheriffbot@chromium.org, Oct 1 2016

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 32 by sheriffbot@chromium.org, Oct 2 2016

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: allpublic

Sign in to add a comment