Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 2 users
Status: Fixed
Owner:
Closed: Mar 2010
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 0
Type: Bug-Security

Restricted
  • Only users with Commit permission may comment.



Sign in to add a comment
WebCore::SVGUseElement::updateContainerOffsets ExecAV@Arbitrary (1dc75f12fe3750aa1828ea20506a5d54)
Reported by aohe...@gmail.com, Mar 1 2010 Back to list
Opening the attached SVG file or a page containing it causes a segmentation
fault in WebCore::SVGUseElement::setHrefBaseValue in 32-bit Ubuntu 9.10.
Builds 36515 up to the current 40259 appear to be affected. In 64-bit
Fedora 12 the issue manifests as the tab getting stuck in loading, sad tab
or a browser crash (segmentation fault after a possible double-free or
memory corruption) when using Google Chrome  5.0.307.9 (Official Build
39052) beta or Chromium 5.0.339.0 (Developer Build 40152).

I have no proof that this is exploitable. Reported as a security issue to
be on the safe side.
 
bad2.svg
133 bytes View Download
svg2-gdb.txt
6.4 KB View Download
Labels: -Area-Undefined Area-WebKit
Status: Available
Summary: WebCore::SVGUseElement::updateContainerOffsets ExecAV@Arbitrary (1dc75f12fe3750aa1828ea20506a5d54) (was: NULL)
Using 5.0.339.0 (40179), I see evidence of an exploitable vulnerability. I tried three times and always got this exception:

Id:          WebCore::SVGUseElement::updateContainerOffsets ExecAV@Arbitrary (1dc75f12fe3750aa1828ea20506a5d54)
Description: Attempt to execute non-executable arbitrary memory @ 0x02895030 in WebCore::SVGUseElement::updateContainerOffsets
Stack:
  [Missing symbols]
  WebCore::SVGUseElement::updateContainerOffsets
  WebCore::SVGUseElement::buildShadowAndInstanceTree
  WebCore::RenderSVGShadowTreeRootContainer::updateFromElement
  WebCore::ContainerNode::dispatchPostAttachCallbacks
  WebCore::ContainerNode::resumePostAttachCallbacks
  WebCore::ContainerNode::appendChild
  WebCore::XMLTokenizer::insertErrorMessageBlock
  WebCore::XMLTokenizer::finish
  WebCore::FrameLoader::endIfNotLoadingMainResource
  WebCore::FrameLoader::finishedLoading
  WebCore::MainResourceLoader::didFinishLoading
  WebCore::ResourceLoader::didFinishLoading
  webkit_glue::WebURLLoaderImpl::Context::OnCompletedRequest
  ResourceDispatcher::OnRequestComplete
  IPC::MessageWithTuple<...><...><...><...><...>::Dispatch<...><...><...><...>
  ResourceDispatcher::DispatchMessageW
  ResourceDispatcher::OnMessageReceived
  ChildThread::OnMessageReceived
  RunnableMethod<...><...>::Run
  MessageLoop::RunTask
  MessageLoop::DoWork
  base::MessagePumpDefault::Run
  MessageLoop::RunInternal
  MessageLoop::Run
  RendererMain

Attached are details grabbed from the debugger.

The repro contains this:

<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
<g id="foo"><use/></g>
<use xlink:href="#foo"/> 

It seems to be that the <use/> tag is the linkely culprit: we are already aware of problems in the code that implements it and a mayor 
rewrite is in progress. I'll try to find the bug for that and mark this as a duplicate if appropriate.
WebCore..SVGUseElement..updateContainerOffsets ExecAV@Arbitrary (1dc75f12fe3750aa1828ea20506a5d54).html
452 KB Download
Labels: SecSeverity-High
Ugh, affects 4.1.249.1021 but not 4.0.249.89
http://crash/reportdetail?reportid=87fbaea255d7f8eb
(In this instance, it's a wild read, but the result of that is clearly used as the 
basis of a wild write so, exploitable.... probably a use after free)
  mov ecx,dword ptr [ecx+4]    (ecx wild!!! 0xabcd1204)
  mov dword ptr <addr>[ecx+esi], <addr>

This is very welcome report. Thank you, Aki.
Labels: Mstone-4.1 ReleaseBlock-Stable
Has anyone tried this with Safari and a WebKit nightly?

It looks like something that would need to get fixed upstream.
Comment 6 by oritm@chromium.org, Mar 3 2010
I can't see the WebKit bug. Is it assigned already and expected to be fixed in the next 
few days...?
It's fixed: Comment #9 From Oliver Hunt 2010-03-04 00:58:33 PST (-) [reply] 
Committed r55511
http://trac.webkit.org/changeset/55511

I tried to add you to the webkit bug as oritm@chromium.org, but apparently you're not 
registered there under that email address, so I can't.
Status: Assigned
Dimitry,

Can you merge this on to the 249 branch?
Dave, I think you're sheriff today, so assigning over to you?

Context: There's a bad security bug that we know about, and we're trying to get a 4.1 
update out today. We would really like to get this patch in the update. Sadly, it 
looks like this patch is causing crashes on the buildbots - dglazkov wasn't sure if 
it was an assert() in the code that was no longer correct, or what.

Is there any chance you can look into this one? Really would like to get this patch 
merged to the 249 branch if possible.
First, it looks like it should be safe to pick up this fix as is. If you need to run debug, then remove the " 
ASSERT(frameCount());" in  ImageSource::frameIsCompleteAtIndex in 
chromium/src/third_party/WebKit/WebCore/platform/graphics/cg/ImageSourceCG.cpp.

Details:

Next, here's how to repro the current issue, run test_shell with full paths to 
  chromium/src/third_party/WebKit/LayoutTests/svg/custom/tiling-regular-hexagonal-crash.svg
  chromium/src/third_party/WebKit/LayoutTests/svg/custom/transform-ignore-after-invalid.svg
The second one crashes. For some reason running the second one alone didn't seem to cause the assert.

The assert occurs is this one:
  bool ImageSource::frameIsCompleteAtIndex(size_t index)
  {
      ASSERT(frameCount());

This happens because the m_decoder is 0 (because the image that was attempted to be created was 156 X 0.

Regardless, the rest of the code in this function handles this nicely and there are no problems. I need to do 
more investigation to understand if the assert can be removed. (When the assert fires, it doesn't seem to be 
harmful, but it does seem to be an unexpected occurrence so in that sense it may be valid.)


Update: WebKit r55169 (which was on Feb 23, 2010) added this assert. Previously very similar code would run 
without doing this assert, so I think the assert was added incorrectly.

Investigation still ongoing.

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=40827 

------------------------------------------------------------------------
r40827 | mal@chromium.org | 2010-03-05 23:16:00 -0800 (Fri, 05 Mar 2010) | 5 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/WebKit/249/LayoutTests/platform/mac/svg/custom/use-empty-reference-expected.txt?r1=40827&r2=40826
   A http://src.chromium.org/viewvc/chrome/branches/WebKit/249/LayoutTests/svg/custom/use-nested-disallowed-target-expected.txt
   A http://src.chromium.org/viewvc/chrome/branches/WebKit/249/LayoutTests/svg/custom/use-nested-disallowed-target.svg
   A http://src.chromium.org/viewvc/chrome/branches/WebKit/249/LayoutTests/svg/custom/use-nested-missing-target-added-expected.txt
   A http://src.chromium.org/viewvc/chrome/branches/WebKit/249/LayoutTests/svg/custom/use-nested-missing-target-added.svg
   A http://src.chromium.org/viewvc/chrome/branches/WebKit/249/LayoutTests/svg/custom/use-nested-missing-target-expected.txt
   A http://src.chromium.org/viewvc/chrome/branches/WebKit/249/LayoutTests/svg/custom/use-nested-missing-target-removed-expected.txt
   A http://src.chromium.org/viewvc/chrome/branches/WebKit/249/LayoutTests/svg/custom/use-nested-missing-target-removed.svg
   A http://src.chromium.org/viewvc/chrome/branches/WebKit/249/LayoutTests/svg/custom/use-nested-missing-target.svg
   A http://src.chromium.org/viewvc/chrome/branches/WebKit/249/LayoutTests/svg/custom/use-nested-notarget-expected.txt
   A http://src.chromium.org/viewvc/chrome/branches/WebKit/249/LayoutTests/svg/custom/use-nested-notarget.svg

Check in test changes for Webkit  bug 35603 .

BUG=  37061 
TEST= yes
Review URL: http://codereview.chromium.org/669236
------------------------------------------------------------------------

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=40828 

------------------------------------------------------------------------
r40828 | mal@chromium.org | 2010-03-05 23:17:44 -0800 (Fri, 05 Mar 2010) | 5 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/WebKit/249/WebCore/svg/SVGUseElement.cpp?r1=40828&r2=40827

Merge WebKit r55511

BUG=  37061 
TEST= see LayoutTests
Review URL: http://codereview.chromium.org/669237
------------------------------------------------------------------------

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=40829 

------------------------------------------------------------------------
r40829 | mal@chromium.org | 2010-03-05 23:23:42 -0800 (Fri, 05 Mar 2010) | 5 lines
Changed paths:
   A http://src.chromium.org/viewvc/chrome/branches/249/src/webkit/data/layout_tests/platform/chromium-win/LayoutTests/svg/custom/relative-sized-deep-shadow-tree-content-expected.checksum
   A http://src.chromium.org/viewvc/chrome/branches/249/src/webkit/data/layout_tests/platform/chromium-win/LayoutTests/svg/custom/relative-sized-deep-shadow-tree-content-expected.png
   A http://src.chromium.org/viewvc/chrome/branches/249/src/webkit/data/layout_tests/platform/chromium-win/LayoutTests/svg/custom/relative-sized-deep-shadow-tree-content-expected.txt
   A http://src.chromium.org/viewvc/chrome/branches/249/src/webkit/data/layout_tests/platform/chromium-win/LayoutTests/svg/custom/relative-sized-shadow-tree-content-expected.checksum
   A http://src.chromium.org/viewvc/chrome/branches/249/src/webkit/data/layout_tests/platform/chromium-win/LayoutTests/svg/custom/relative-sized-shadow-tree-content-expected.png
   A http://src.chromium.org/viewvc/chrome/branches/249/src/webkit/data/layout_tests/platform/chromium-win/LayoutTests/svg/custom/relative-sized-shadow-tree-content-expected.txt
   A http://src.chromium.org/viewvc/chrome/branches/249/src/webkit/data/layout_tests/platform/chromium-win-vista/LayoutTests/svg/custom/use-empty-reference-expected.checksum
   A http://src.chromium.org/viewvc/chrome/branches/249/src/webkit/data/layout_tests/platform/chromium-win-vista/LayoutTests/svg/custom/use-empty-reference-expected.png
   A http://src.chromium.org/viewvc/chrome/branches/249/src/webkit/data/layout_tests/platform/chromium-win-vista/LayoutTests/svg/custom/use-empty-reference-expected.txt
   A http://src.chromium.org/viewvc/chrome/branches/249/src/webkit/data/layout_tests/platform/chromium-win-vista/LayoutTests/svg/custom/use-nested-missing-target-removed-expected.txt

Update expectations for WebKit r55511

BUG=  37061 
TEST= yes
Review URL: http://codereview.chromium.org/669239
------------------------------------------------------------------------

Status: FixUnreleased
Merged r55511, which turns the originally attached svg file from a sad tab into a happy 
tab. (Verified on a local 249.1026 build.)

No new crashes running layout tests.
Labels: Reward-500
@aohelin: congrats! Subject to the usual continued responsible disclosure, we wish to 
offer you a $500 reward. You can e-mail me at cevans@chromium.org to let me know if 
you accept or not.

@mal: is there a dedicated graphic we use for a "happy tab"? :D
Labels: -Restrict-View-SecurityTeam
Status: Fixed
Releasing due to fix in 4.1.249.1036.
Labels: Type-Security
Labels: SecImpacts-None
Batch update.
Project Member Comment 20 by bugdroid1@chromium.org, Oct 13 2012
Labels: Restrict-AddIssueComment-Commit
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 21 by bugdroid1@chromium.org, Mar 10 2013
Labels: -Area-WebKit -SecSeverity-High -Type-Security -SecImpacts-None Cr-Content Security-Impact-None Security-Severity-High Type-Bug-Security
Project Member Comment 22 by bugdroid1@chromium.org, Mar 21 2013
Labels: Security_Severity-None
Project Member Comment 23 by bugdroid1@chromium.org, Mar 21 2013
Labels: -Security-Impact-None Security_Impact-None
Project Member Comment 24 by bugdroid1@chromium.org, Mar 21 2013
Labels: -Security-Severity-High -Security_Severity-None Security_Severity-High
Project Member Comment 25 by bugdroid1@chromium.org, Apr 6 2013
Labels: -Cr-Content Cr-Blink
Labels: allpublic
Sign in to add a comment