Stop storing raw pointers to Documents
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 25 Mar 2019 21:21:50 +0000 (21:21 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 25 Mar 2019 21:21:50 +0000 (21:21 +0000)
https://bugs.webkit.org/show_bug.cgi?id=196042

Reviewed by Geoff Garen.

Use WeakPtr instead!  This could change some UAF bugs into null dereference crashes.

* css/CSSFontSelector.cpp:
(WebCore::CSSFontSelector::CSSFontSelector):
(WebCore::CSSFontSelector::addFontFaceRule):
(WebCore::CSSFontSelector::fontRangesForFamily):
* css/CSSFontSelector.h:
* css/MediaQueryMatcher.cpp:
(WebCore::MediaQueryMatcher::MediaQueryMatcher):
(WebCore::MediaQueryMatcher::matchMedia):
* css/MediaQueryMatcher.h:
* css/StyleSheetList.cpp:
(WebCore::StyleSheetList::StyleSheetList):
(WebCore::StyleSheetList::ownerNode const):
* css/StyleSheetList.h:
* css/ViewportStyleResolver.cpp:
(WebCore::ViewportStyleResolver::ViewportStyleResolver):
* css/ViewportStyleResolver.h:
* dom/Document.h:
(WebCore::Document::setTemplateDocumentHost):
(WebCore::Document::templateDocumentHost):
* dom/DocumentParser.cpp:
(WebCore::DocumentParser::DocumentParser):
* dom/DocumentParser.h:
(WebCore::DocumentParser::document const):
* dom/ScriptedAnimationController.cpp:
(WebCore::ScriptedAnimationController::ScriptedAnimationController):
* dom/ScriptedAnimationController.h:
* html/parser/HTMLScriptRunner.cpp:
(WebCore::HTMLScriptRunner::HTMLScriptRunner):
(WebCore::HTMLScriptRunner::runScript):
* html/parser/HTMLScriptRunner.h:
* loader/MediaResourceLoader.cpp:
(WebCore::MediaResourceLoader::MediaResourceLoader):
* loader/MediaResourceLoader.h:
* loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::canRequestInContentDispositionAttachmentSandbox const):
(WebCore::CachedResourceLoader::loadDone):
* loader/cache/CachedResourceLoader.h:
(WebCore::CachedResourceLoader::document const):
(WebCore::CachedResourceLoader::setDocument):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@243459 268f45cc-cd09-0410-ab3c-d52691b4dbfc

20 files changed:
Source/WebCore/ChangeLog
Source/WebCore/css/CSSFontSelector.cpp
Source/WebCore/css/CSSFontSelector.h
Source/WebCore/css/MediaQueryMatcher.cpp
Source/WebCore/css/MediaQueryMatcher.h
Source/WebCore/css/StyleSheetList.cpp
Source/WebCore/css/StyleSheetList.h
Source/WebCore/css/ViewportStyleResolver.cpp
Source/WebCore/css/ViewportStyleResolver.h
Source/WebCore/dom/Document.h
Source/WebCore/dom/DocumentParser.cpp
Source/WebCore/dom/DocumentParser.h
Source/WebCore/dom/ScriptedAnimationController.cpp
Source/WebCore/dom/ScriptedAnimationController.h
Source/WebCore/html/parser/HTMLScriptRunner.cpp
Source/WebCore/html/parser/HTMLScriptRunner.h
Source/WebCore/loader/MediaResourceLoader.cpp
Source/WebCore/loader/MediaResourceLoader.h
Source/WebCore/loader/cache/CachedResourceLoader.cpp
Source/WebCore/loader/cache/CachedResourceLoader.h

index a9574f5..f512d79 100644 (file)
@@ -1,3 +1,52 @@
+2019-03-25  Alex Christensen  <achristensen@webkit.org>
+
+        Stop storing raw pointers to Documents
+        https://bugs.webkit.org/show_bug.cgi?id=196042
+
+        Reviewed by Geoff Garen.
+
+        Use WeakPtr instead!  This could change some UAF bugs into null dereference crashes.
+
+        * css/CSSFontSelector.cpp:
+        (WebCore::CSSFontSelector::CSSFontSelector):
+        (WebCore::CSSFontSelector::addFontFaceRule):
+        (WebCore::CSSFontSelector::fontRangesForFamily):
+        * css/CSSFontSelector.h:
+        * css/MediaQueryMatcher.cpp:
+        (WebCore::MediaQueryMatcher::MediaQueryMatcher):
+        (WebCore::MediaQueryMatcher::matchMedia):
+        * css/MediaQueryMatcher.h:
+        * css/StyleSheetList.cpp:
+        (WebCore::StyleSheetList::StyleSheetList):
+        (WebCore::StyleSheetList::ownerNode const):
+        * css/StyleSheetList.h:
+        * css/ViewportStyleResolver.cpp:
+        (WebCore::ViewportStyleResolver::ViewportStyleResolver):
+        * css/ViewportStyleResolver.h:
+        * dom/Document.h:
+        (WebCore::Document::setTemplateDocumentHost):
+        (WebCore::Document::templateDocumentHost):
+        * dom/DocumentParser.cpp:
+        (WebCore::DocumentParser::DocumentParser):
+        * dom/DocumentParser.h:
+        (WebCore::DocumentParser::document const):
+        * dom/ScriptedAnimationController.cpp:
+        (WebCore::ScriptedAnimationController::ScriptedAnimationController):
+        * dom/ScriptedAnimationController.h:
+        * html/parser/HTMLScriptRunner.cpp:
+        (WebCore::HTMLScriptRunner::HTMLScriptRunner):
+        (WebCore::HTMLScriptRunner::runScript):
+        * html/parser/HTMLScriptRunner.h:
+        * loader/MediaResourceLoader.cpp:
+        (WebCore::MediaResourceLoader::MediaResourceLoader):
+        * loader/MediaResourceLoader.h:
+        * loader/cache/CachedResourceLoader.cpp:
+        (WebCore::CachedResourceLoader::canRequestInContentDispositionAttachmentSandbox const):
+        (WebCore::CachedResourceLoader::loadDone):
+        * loader/cache/CachedResourceLoader.h:
+        (WebCore::CachedResourceLoader::document const):
+        (WebCore::CachedResourceLoader::setDocument):
+
 2019-03-25  Truitt Savell  <tsavell@apple.com>
 
         Unreviewed, rolling out r243419.
index 4afb42e..a754685 100644 (file)
@@ -62,7 +62,7 @@ namespace WebCore {
 static unsigned fontSelectorId;
 
 CSSFontSelector::CSSFontSelector(Document& document)
-    : m_document(&document)
+    : m_document(makeWeakPtr(document))
     , m_cssFontFaceSet(CSSFontFaceSet::create(this))
     , m_beginLoadingTimer(*this, &CSSFontSelector::beginLoadTimerFired)
     , m_uniqueId(++fontSelectorId)
@@ -207,7 +207,7 @@ void CSSFontSelector::addFontFaceRule(StyleRuleFontFace& fontFaceRule, bool isIn
     if (loadingBehavior)
         fontFace->setLoadingBehavior(*loadingBehavior);
 
-    CSSFontFace::appendSources(fontFace, srcList, m_document, isInitiatingElementInUserAgentShadowTree);
+    CSSFontFace::appendSources(fontFace, srcList, m_document.get(), isInitiatingElementInUserAgentShadowTree);
     if (fontFace->computeFailureState())
         return;
 
@@ -312,7 +312,7 @@ FontRanges CSSFontSelector::fontRangesForFamily(const FontDescription& fontDescr
     // FIXME: The spec (and Firefox) says user specified generic families (sans-serif etc.) should be resolved before the @font-face lookup too.
     bool resolveGenericFamilyFirst = familyName == standardFamily;
 
-    AtomicString familyForLookup = resolveGenericFamilyFirst ? resolveGenericFamily(m_document, fontDescription, familyName) : familyName;
+    AtomicString familyForLookup = resolveGenericFamilyFirst ? resolveGenericFamily(m_document.get(), fontDescription, familyName) : familyName;
     auto* face = m_cssFontFaceSet->fontFace(fontDescription.fontSelectionRequest(), familyForLookup);
     if (face) {
         if (RuntimeEnabledFeatures::sharedFeatures().webAPIStatisticsEnabled()) {
@@ -322,7 +322,7 @@ FontRanges CSSFontSelector::fontRangesForFamily(const FontDescription& fontDescr
         return face->fontRanges(fontDescription);
     }
     if (!resolveGenericFamilyFirst)
-        familyForLookup = resolveGenericFamily(m_document, fontDescription, familyName);
+        familyForLookup = resolveGenericFamily(m_document.get(), fontDescription, familyName);
     auto font = FontCache::singleton().fontForFamily(fontDescription, familyForLookup);
     if (RuntimeEnabledFeatures::sharedFeatures().webAPIStatisticsEnabled()) {
         if (m_document)
index ad52f05..eb4e063 100644 (file)
@@ -77,7 +77,7 @@ public:
     void registerForInvalidationCallbacks(FontSelectorClient&) final;
     void unregisterForInvalidationCallbacks(FontSelectorClient&) final;
 
-    Document* document() const { return m_document; }
+    Document* document() const { return m_document.get(); }
 
     void beginLoadingFontSoon(CachedFont&);
 
@@ -103,7 +103,7 @@ private:
     };
     Vector<PendingFontFaceRule> m_stagingArea;
 
-    Document* m_document;
+    WeakPtr<Document> m_document;
     RefPtr<FontFaceSet> m_fontFaceSet;
     Ref<CSSFontFaceSet> m_cssFontFaceSet;
     HashSet<FontSelectorClient*> m_clients;
index a60344c..52a5589 100644 (file)
@@ -38,7 +38,7 @@
 namespace WebCore {
 
 MediaQueryMatcher::MediaQueryMatcher(Document& document)
-    : m_document(&document)
+    : m_document(makeWeakPtr(document))
 {
 }
 
@@ -84,7 +84,7 @@ RefPtr<MediaQueryList> MediaQueryMatcher::matchMedia(const String& query)
         return nullptr;
 
     auto media = MediaQuerySet::create(query, MediaQueryParserContext(*m_document));
-    reportMediaQueryWarningIfNeeded(m_document, media.ptr());
+    reportMediaQueryWarningIfNeeded(m_document.get(), media.ptr());
     bool result = evaluate(media.get());
     return MediaQueryList::create(*this, WTFMove(media), result);
 }
index 7510710..965073c 100644 (file)
@@ -66,7 +66,7 @@ private:
     std::unique_ptr<RenderStyle> documentElementUserAgentStyle() const;
     String mediaType() const;
 
-    Document* m_document;
+    WeakPtr<Document> m_document;
     Vector<Listener> m_listeners;
 
     // This value is incremented at style selector changes.
index f211dac..86932d2 100644 (file)
@@ -34,7 +34,7 @@ namespace WebCore {
 using namespace HTMLNames;
 
 StyleSheetList::StyleSheetList(Document& document)
-    : m_document(&document)
+    : m_document(makeWeakPtr(document))
 {
 }
 
@@ -57,7 +57,7 @@ inline const Vector<RefPtr<StyleSheet>>& StyleSheetList::styleSheets() const
 Node* StyleSheetList::ownerNode() const
 {
     if (m_document)
-        return m_document;
+        return m_document.get();
     return m_shadowRoot;
 }
 
index 8a8fd62..4802571 100644 (file)
@@ -23,6 +23,7 @@
 #include <wtf/Forward.h>
 #include <wtf/RefCounted.h>
 #include <wtf/Vector.h>
+#include <wtf/WeakPtr.h>
 
 namespace WebCore {
 
@@ -54,7 +55,7 @@ private:
     StyleSheetList(ShadowRoot&);
     const Vector<RefPtr<StyleSheet>>& styleSheets() const;
 
-    Document* m_document { nullptr };
+    WeakPtr<Document> m_document;
     ShadowRoot* m_shadowRoot { nullptr };
     Vector<RefPtr<StyleSheet>> m_detachedStyleSheets;
 };
index 1245c8f..cfc416c 100644 (file)
@@ -42,7 +42,7 @@
 namespace WebCore {
 
 ViewportStyleResolver::ViewportStyleResolver(Document* document)
-    : m_document(document)
+    : m_document(document ? makeWeakPtr(*document) : nullptr)
 {
     ASSERT(m_document);
 }
index c431271..e1b8e5a 100644 (file)
@@ -61,7 +61,7 @@ private:
 
     float getViewportArgumentValue(CSSPropertyID) const;
 
-    Document* m_document;
+    WeakPtr<Document> m_document;
     RefPtr<MutableStyleProperties> m_propertySet;
 };
 
index 57714d7..670fa05 100644 (file)
@@ -1344,8 +1344,8 @@ public:
 
     const Document* templateDocument() const;
     Document& ensureTemplateDocument();
-    void setTemplateDocumentHost(Document* templateDocumentHost) { m_templateDocumentHost = templateDocumentHost; }
-    Document* templateDocumentHost() { return m_templateDocumentHost; }
+    void setTemplateDocumentHost(Document* templateDocumentHost) { m_templateDocumentHost = makeWeakPtr(templateDocumentHost); }
+    Document* templateDocumentHost() { return m_templateDocumentHost.get(); }
 
     void didAssociateFormControl(Element&);
     bool hasDisabledFieldsetElement() const { return m_disabledFieldsetElementsCount; }
@@ -1923,7 +1923,7 @@ private:
     LocaleIdentifierToLocaleMap m_localeCache;
 
     RefPtr<Document> m_templateDocument;
-    Document* m_templateDocumentHost { nullptr }; // Manually managed weakref (backpointer from m_templateDocument).
+    WeakPtr<Document> m_templateDocumentHost; // Manually managed weakref (backpointer from m_templateDocument).
 
     Ref<CSSFontSelector> m_fontSelector;
 
index 6bcfb00..302addf 100644 (file)
@@ -33,7 +33,7 @@ namespace WebCore {
 DocumentParser::DocumentParser(Document& document)
     : m_state(ParsingState)
     , m_documentWasLoadedAsPartOfNavigation(false)
-    , m_document(&document)
+    , m_document(makeWeakPtr(document))
 {
 }
 
index 85d6209..ddeaf2f 100644 (file)
@@ -62,7 +62,7 @@ public:
     virtual bool processingData() const { return false; }
 
     // document() will return 0 after detach() is called.
-    Document* document() const { ASSERT(m_document); return m_document; }
+    Document* document() const { ASSERT(m_document); return m_document.get(); }
 
     bool isParsing() const { return m_state == ParsingState; }
     bool isStopping() const { return m_state == StoppingState; }
@@ -114,7 +114,7 @@ private:
 
     // Every DocumentParser needs a pointer back to the document.
     // m_document will be 0 after the parser is stopped.
-    Document* m_document;
+    WeakPtr<Document> m_document;
 };
 
 } // namespace WebCore
index e88bb90..d0f10a1 100644 (file)
@@ -55,7 +55,7 @@ static const Seconds aggressiveThrottlingAnimationInterval { 10_s };
 namespace WebCore {
 
 ScriptedAnimationController::ScriptedAnimationController(Document& document)
-    : m_document(&document)
+    : m_document(makeWeakPtr(document))
     , m_animationTimer(*this, &ScriptedAnimationController::animationTimerFired)
 {
 }
index 7ab8ec1..f08556f 100644 (file)
@@ -84,7 +84,7 @@ private:
     typedef Vector<RefPtr<RequestAnimationFrameCallback>> CallbackList;
     CallbackList m_callbacks;
 
-    Document* m_document;
+    WeakPtr<Document> m_document;
     CallbackId m_nextCallbackId { 0 };
     int m_suspendCount { 0 };
 
index a246454..eb925d8 100644 (file)
@@ -47,7 +47,7 @@ namespace WebCore {
 using namespace HTMLNames;
 
 HTMLScriptRunner::HTMLScriptRunner(Document& document, HTMLScriptRunnerHost& host)
-    : m_document(&document)
+    : m_document(makeWeakPtr(document))
     , m_host(host)
     , m_scriptNestingLevel(0)
     , m_hasScriptsWaitingForStylesheets(false)
@@ -258,7 +258,7 @@ void HTMLScriptRunner::runScript(ScriptElement& scriptElement, const TextPositio
         if (m_scriptNestingLevel == 1)
             m_parserBlockingScript = PendingScript::create(scriptElement, scriptStartPosition);
         else
-            scriptElement.executeClassicScript(ScriptSourceCode(scriptElement.element().textContent(), documentURLForScriptExecution(m_document), scriptStartPosition, JSC::SourceProviderSourceType::Program, InlineClassicScript::create(scriptElement)));
+            scriptElement.executeClassicScript(ScriptSourceCode(scriptElement.element().textContent(), documentURLForScriptExecution(m_document.get()), scriptStartPosition, JSC::SourceProviderSourceType::Program, InlineClassicScript::create(scriptElement)));
     } else
         requestParsingBlockingScript(scriptElement);
 }
index c86747d..e65dc47 100644 (file)
@@ -28,6 +28,7 @@
 
 #include "PendingScript.h"
 #include <wtf/Deque.h>
+#include <wtf/WeakPtr.h>
 #include <wtf/text/TextPosition.h>
 
 namespace WebCore {
@@ -71,7 +72,7 @@ private:
     void stopWatchingForLoad(PendingScript&);
     bool isPendingScriptReady(const PendingScript&);
 
-    Document* m_document;
+    WeakPtr<Document> m_document;
     HTMLScriptRunnerHost& m_host;
     RefPtr<PendingScript> m_parserBlockingScript;
     Deque<Ref<PendingScript>> m_scriptsToExecuteAfterParsing; // http://www.whatwg.org/specs/web-apps/current-work/#list-of-scripts-that-will-execute-when-the-document-has-finished-parsing
index 5deb5c5..db29143 100644 (file)
@@ -43,7 +43,7 @@ namespace WebCore {
 
 MediaResourceLoader::MediaResourceLoader(Document& document, HTMLMediaElement& mediaElement, const String& crossOriginMode)
     : ContextDestructionObserver(&document)
-    , m_document(&document)
+    , m_document(makeWeakPtr(document))
     , m_mediaElement(makeWeakPtr(mediaElement))
     , m_crossOriginMode(crossOriginMode)
 {
index 1b0e00b..1a3e0af 100644 (file)
@@ -52,7 +52,7 @@ public:
     RefPtr<PlatformMediaResource> requestResource(ResourceRequest&&, LoadOptions) final;
     void removeResource(MediaResource&);
 
-    Document* document() { return m_document; }
+    Document* document() { return m_document.get(); }
     const String& crossOriginMode() const { return m_crossOriginMode; }
 
     Vector<ResourceResponse> responsesForTesting() const { return m_responsesForTesting; }
@@ -61,7 +61,7 @@ public:
 private:
     void contextDestroyed() override;
 
-    Document* m_document;
+    WeakPtr<Document> m_document;
     WeakPtr<HTMLMediaElement> m_mediaElement;
     String m_crossOriginMode;
     HashSet<MediaResource*> m_resources;
index 26aa729..b65cf17 100644 (file)
@@ -578,7 +578,7 @@ bool CachedResourceLoader::canRequestInContentDispositionAttachmentSandbox(Cache
         }
         return true;
     case CachedResource::Type::CSSStyleSheet:
-        document = m_document;
+        document = m_document.get();
         break;
     default:
         return true;
@@ -1303,7 +1303,7 @@ CachePolicy CachedResourceLoader::cachePolicy(CachedResource::Type type, const U
 void CachedResourceLoader::loadDone(LoadCompletionType type, bool shouldPerformPostLoadActions)
 {
     RefPtr<DocumentLoader> protectDocumentLoader(m_documentLoader);
-    RefPtr<Document> protectDocument(m_document);
+    RefPtr<Document> protectDocument(m_document.get());
 
     ASSERT(shouldPerformPostLoadActions || type == LoadCompletionType::Cancel);
 
index d5248cc..0f6d600 100644 (file)
@@ -125,8 +125,8 @@ public:
     CachePolicy cachePolicy(CachedResource::Type, const URL&) const;
     
     Frame* frame() const; // Can be null
-    Document* document() const { return m_document; } // Can be null
-    void setDocument(Document* document) { m_document = document; }
+    Document* document() const { return m_document.get(); } // Can be null
+    void setDocument(Document* document) { m_document = makeWeakPtr(document); }
     void clearDocumentLoader() { m_documentLoader = nullptr; }
     PAL::SessionID sessionID() const;
 
@@ -193,7 +193,7 @@ private:
 
     HashSet<String> m_validatedURLs;
     mutable DocumentResourceMap m_documentResources;
-    Document* m_document;
+    WeakPtr<Document> m_document;
     DocumentLoader* m_documentLoader;
 
     int m_requestCount;