Skip to content

Commit

Permalink
Cherry-pick 259548.186@safari-7615-branch (1aa5ac3). rdar://107561498
Browse files Browse the repository at this point in the history
    Use WeakPtr to track resources in SubresourceLoader
    https://bugs.webkit.org/show_bug.cgi?id=252200
    rdar://104908871

    Reviewed by Chris Dumez.

    CachedResource can get deallocated when the SubresourceLoader releases
    its resources, in which case we'd be accessing a dangling pointer. This
    change adopts WeakPtr for CachedResource so that we don't have a UAF.

    * Source/WebCore/loader/SubresourceLoader.cpp:
    (WebCore::SubresourceLoader::didFinishLoading):
    (WebCore::SubresourceLoader::didFail):
    * Source/WebCore/loader/SubresourceLoader.h:
    * Source/WebCore/loader/cache/CachedResource.h:
    * Source/WebCore/loader/cache/CachedResourceLoader.h:

    Canonical link: https://commits.webkit.org/259548.186@safari-7615-branch

Canonical link: https://commits.webkit.org/262538@main
  • Loading branch information
chirags27 authored and EWS Buildbot Worker committed Apr 3, 2023
1 parent 2deb5a6 commit 5c3b82d
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 7 deletions.
4 changes: 2 additions & 2 deletions Source/WebCore/loader/SubresourceLoader.cpp
Expand Up @@ -731,7 +731,7 @@ void SubresourceLoader::didFinishLoading(const NetworkLoadMetrics& networkLoadMe
logResourceLoaded(m_frame.get(), m_resource->type());

Ref<SubresourceLoader> protectedThis(*this);
CachedResourceHandle<CachedResource> protectResource(m_resource);
CachedResourceHandle<CachedResource> protectResource(m_resource.get());

m_loadTiming.markEndTime();

Expand Down Expand Up @@ -788,7 +788,7 @@ void SubresourceLoader::didFail(const ResourceError& error)
m_frame->document()->addConsoleMessage(MessageSource::Security, MessageLevel::Error, error.localizedDescription());

Ref<SubresourceLoader> protectedThis(*this);
CachedResourceHandle<CachedResource> protectResource(m_resource);
CachedResourceHandle<CachedResource> protectResource(m_resource.get());
m_state = Finishing;

if (m_resource->type() != CachedResource::Type::MainResource)
Expand Down
9 changes: 5 additions & 4 deletions Source/WebCore/loader/SubresourceLoader.h
Expand Up @@ -32,6 +32,7 @@
#include "FrameLoaderTypes.h"
#include "ResourceLoader.h"
#include <wtf/CompletionHandler.h>
#include <wtf/WeakPtr.h>
#include <wtf/text/WTFString.h>

namespace WebCore {
Expand All @@ -50,7 +51,7 @@ class SubresourceLoader final : public ResourceLoader {

void cancelIfNotFinishing();
bool isSubresourceLoader() const override;
CachedResource* cachedResource() const override { return m_resource; };
CachedResource* cachedResource() const override { return m_resource.get(); };
WEBCORE_EXPORT const HTTPHeaderMap* originalHeaders() const;

const SecurityOrigin* origin() const { return m_origin.get(); }
Expand Down Expand Up @@ -126,14 +127,14 @@ class SubresourceLoader final : public ResourceLoader {
RequestCountTracker& operator=(RequestCountTracker&&);
~RequestCountTracker();
private:
CachedResourceLoader* m_cachedResourceLoader { nullptr };
const CachedResource* m_resource { nullptr };
WeakPtr<CachedResourceLoader> m_cachedResourceLoader;
WeakPtr<const CachedResource> m_resource;
};

#if PLATFORM(IOS_FAMILY)
ResourceRequest m_iOSOriginalRequest;
#endif
CachedResource* m_resource;
WeakPtr<CachedResource> m_resource;
SubresourceLoaderState m_state;
std::optional<RequestCountTracker> m_requestCountTracker;
RefPtr<SecurityOrigin> m_origin;
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/loader/cache/CachedResourceLoader.h
Expand Up @@ -76,7 +76,7 @@ enum class FetchMetadataSite : uint8_t { None, SameOrigin, SameSite, CrossSite }
// RefPtr<CachedResourceLoader> for their lifetime (and will create one if they
// are initialized without a Frame), so a Document can keep a CachedResourceLoader
// alive past detach if scripts still reference the Document.
class CachedResourceLoader : public RefCounted<CachedResourceLoader> {
class CachedResourceLoader : public RefCounted<CachedResourceLoader>, public CanMakeWeakPtr<CachedResourceLoader> {
WTF_MAKE_NONCOPYABLE(CachedResourceLoader); WTF_MAKE_FAST_ALLOCATED;
friend class ImageLoader;
friend class ResourceCacheValidationSuppressor;
Expand Down

0 comments on commit 5c3b82d

Please sign in to comment.