From 305284960db83fe9b9ae47674db9914d82180c23 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Thu, 3 Sep 2015 10:34:36 +0200 Subject: Cherry-pick fix for CVE-2015-1284 Fix the logic that limits the number of frames in a page. This check apparently doesn't run soon enough, and we can create more than the intended limit of 1000 frames. Once we hit 1024, NodeRareData::m_connecetedFrameCount can overflow and we no longer fully detach Frames from their owners at teardown. BUG=493243 TEST=WebFrameTest.MaxFramesDetach Review URL: https://codereview.chromium.org/1180603002 Change-Id: Ib83a10c6c9cece32c39aed0cbbb494522c5eb3dd Reviewed-by: Kai Koehne --- chromium/third_party/WebKit/Source/core/dom/NodeRareData.cpp | 6 ++++++ chromium/third_party/WebKit/Source/core/dom/NodeRareData.h | 5 +---- chromium/third_party/WebKit/Source/core/frame/LocalFrame.cpp | 2 -- .../third_party/WebKit/Source/core/html/HTMLFrameOwnerElement.cpp | 4 ++++ 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/chromium/third_party/WebKit/Source/core/dom/NodeRareData.cpp b/chromium/third_party/WebKit/Source/core/dom/NodeRareData.cpp index 1f0489a8ee9..26b01415117 100644 --- a/chromium/third_party/WebKit/Source/core/dom/NodeRareData.cpp +++ b/chromium/third_party/WebKit/Source/core/dom/NodeRareData.cpp @@ -76,6 +76,12 @@ void NodeRareData::finalizeGarbageCollectedObject() this->~NodeRareData(); } +void NodeRareData::incrementConnectedSubframeCount(unsigned amount) +{ + RELEASE_ASSERT_WITH_SECURITY_IMPLICATION((m_connectedFrameCount + amount) <= FrameHost::maxNumberOfFrames); + m_connectedFrameCount += amount; +} + // Ensure the 10 bits reserved for the m_connectedFrameCount cannot overflow static_assert(FrameHost::maxNumberOfFrames < (1 << NodeRareData::ConnectedFrameCountBits), "Frame limit should fit in rare data count"); diff --git a/chromium/third_party/WebKit/Source/core/dom/NodeRareData.h b/chromium/third_party/WebKit/Source/core/dom/NodeRareData.h index 6c661b5a790..2df091bad3b 100644 --- a/chromium/third_party/WebKit/Source/core/dom/NodeRareData.h +++ b/chromium/third_party/WebKit/Source/core/dom/NodeRareData.h @@ -82,10 +82,7 @@ public: } unsigned connectedSubframeCount() const { return m_connectedFrameCount; } - void incrementConnectedSubframeCount(unsigned amount) - { - m_connectedFrameCount += amount; - } + void incrementConnectedSubframeCount(unsigned amount); void decrementConnectedSubframeCount(unsigned amount) { ASSERT(m_connectedFrameCount); diff --git a/chromium/third_party/WebKit/Source/core/frame/LocalFrame.cpp b/chromium/third_party/WebKit/Source/core/frame/LocalFrame.cpp index 9b5c504e0f6..e5d7d7b423c 100644 --- a/chromium/third_party/WebKit/Source/core/frame/LocalFrame.cpp +++ b/chromium/third_party/WebKit/Source/core/frame/LocalFrame.cpp @@ -707,8 +707,6 @@ bool LocalFrame::isURLAllowed(const KURL& url) const { // We allow one level of self-reference because some sites depend on that, // but we don't allow more than one. - if (host()->subframeCount() >= FrameHost::maxNumberOfFrames) - return false; bool foundSelfReference = false; for (const Frame* frame = this; frame; frame = frame->tree().parent()) { if (!frame->isLocalFrame()) diff --git a/chromium/third_party/WebKit/Source/core/html/HTMLFrameOwnerElement.cpp b/chromium/third_party/WebKit/Source/core/html/HTMLFrameOwnerElement.cpp index 48327673b3e..c7481b8176b 100644 --- a/chromium/third_party/WebKit/Source/core/html/HTMLFrameOwnerElement.cpp +++ b/chromium/third_party/WebKit/Source/core/html/HTMLFrameOwnerElement.cpp @@ -26,6 +26,7 @@ #include "core/accessibility/AXObjectCache.h" #include "core/dom/ExceptionCode.h" #include "core/events/Event.h" +#include "core/frame/FrameHost.h" #include "core/frame/FrameView.h" #include "core/frame/LocalFrame.h" #include "core/loader/FrameLoader.h" @@ -254,6 +255,9 @@ bool HTMLFrameOwnerElement::loadOrRedirectSubframe(const KURL& url, const Atomic if (!SubframeLoadingDisabler::canLoadFrame(*this)) return false; + if (document().frame()->host()->subframeCount() >= FrameHost::maxNumberOfFrames) + return false; + return parentFrame->loader().client()->createFrame(url, frameName, this); } -- cgit v1.2.3