Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2021)

Unified Diff: Source/web/WebLocalFrameImpl.cpp

Issue 339513002: Fix use-after-free when creating and detaching iframe during load. (Closed) Base URL: svn://svn.chromium.org/blink/trunk
Patch Set: Remove test since this is already covered by multiple, similar tests. Created 6 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « Source/web/WebLocalFrameImpl.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: Source/web/WebLocalFrameImpl.cpp
diff --git a/Source/web/WebLocalFrameImpl.cpp b/Source/web/WebLocalFrameImpl.cpp
index f08b9a1da0a82a5ea5579543efd56a4c40184424..6b8f5c155d785b58d9277b4f807aad497e1d4ee4 100644
--- a/Source/web/WebLocalFrameImpl.cpp
+++ b/Source/web/WebLocalFrameImpl.cpp
@@ -1670,35 +1670,36 @@ void WebLocalFrameImpl::initializeAsMainFrame(WebCore::Page* page)
PassRefPtr<LocalFrame> WebLocalFrameImpl::createChildFrame(const FrameLoadRequest& request, HTMLFrameOwnerElement* ownerElement)
{
ASSERT(m_client);
- // Protect a reference to the new child frame, in case it gets detached.
- RefPtr<WebLocalFrameImpl> child = toWebLocalFrameImpl(m_client->createChildFrame(this, request.frameName()));
- if (!child)
+ WebLocalFrameImpl* webframeChild = toWebLocalFrameImpl(m_client->createChildFrame(this, request.frameName()));
+ if (!webframeChild)
return nullptr;
// FIXME: Using subResourceAttributeName as fallback is not a perfect
// solution. subResourceAttributeName returns just one attribute name. The
// element might not have the attribute, and there might be other attributes
// which can identify the element.
- child->initializeAsChildFrame(frame()->host(), ownerElement, request.frameName(), ownerElement->getAttribute(ownerElement->subResourceAttributeName()));
+ RefPtr<LocalFrame> child = webframeChild->initializeAsChildFrame(frame()->host(), ownerElement, request.frameName(), ownerElement->getAttribute(ownerElement->subResourceAttributeName()));
// Initializing the WebCore frame may cause the new child to be detached, since it may dispatch a load event in the parent.
- if (!child->frame())
+ if (!child->tree().parent())
return nullptr;
// If we're moving in the back/forward list, we might want to replace the content
// of this child frame with whatever was there at that point.
RefPtr<HistoryItem> childItem;
if (isBackForwardLoadType(frame()->loader().loadType()) && !frame()->document()->loadEventFinished())
- childItem = PassRefPtr<HistoryItem>(child->client()->historyItemForNewChildFrame(child.get()));
+ childItem = PassRefPtr<HistoryItem>(webframeChild->client()->historyItemForNewChildFrame(webframeChild));
if (childItem)
- child->frame()->loader().loadHistoryItem(childItem.get());
+ child->loader().loadHistoryItem(childItem.get());
else
- child->frame()->loader().load(FrameLoadRequest(0, request.resourceRequest(), "_self"));
+ child->loader().load(FrameLoadRequest(0, request.resourceRequest(), "_self"));
// Note a synchronous navigation (about:blank) would have already processed
- // onload, so it is possible for the child frame to have already been destroyed by
- // script in the page.
- return child->frame();
+ // onload, so it is possible for the child frame to have already been
+ // detached by script in the page.
+ if (!child->tree().parent())
+ return nullptr;
+ return child;
}
void WebLocalFrameImpl::didChangeContentsSize(const IntSize& size)
@@ -1944,12 +1945,14 @@ void WebLocalFrameImpl::invalidateAll() const
invalidateScrollbar();
}
-void WebLocalFrameImpl::initializeAsChildFrame(FrameHost* host, FrameOwner* owner, const AtomicString& name, const AtomicString& fallbackName)
+PassRefPtr<LocalFrame> WebLocalFrameImpl::initializeAsChildFrame(FrameHost* host, FrameOwner* owner, const AtomicString& name, const AtomicString& fallbackName)
{
- setWebCoreFrame(LocalFrame::create(&m_frameLoaderClientImpl, host, owner));
- frame()->tree().setName(name, fallbackName);
- // May dispatch JS events; frame() may be null after this.
- frame()->init();
+ RefPtr<LocalFrame> frame = LocalFrame::create(&m_frameLoaderClientImpl, host, owner);
+ setWebCoreFrame(frame);
+ frame->tree().setName(name, fallbackName);
+ // May dispatch JS events; frame may be detached after this.
+ frame->init();
+ return frame;
}
} // namespace blink
« no previous file with comments | « Source/web/WebLocalFrameImpl.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698