Skip to content

Commit

Permalink
Cherry-pick 4c34308. rdar://118267012
Browse files Browse the repository at this point in the history
    Crash under PAL::newTextCodec(PAL::TextEncoding const&)
    https://bugs.webkit.org/show_bug.cgi?id=264979
    rdar://118267012

    Reviewed by Brent Fulgham.

    There is evidence for crashes in the wild that the CachedCSSStyleSheet or
    the TextResourceDecoder are being used after getting freed. To prevent this,
    protect both these objects in the code path identified by the crashes.

    This is a speculative fix but it should be very safe.

    * Source/WebCore/loader/SubresourceLoader.cpp:
    (WebCore::SubresourceLoader::didFinishLoading):
    * Source/WebCore/loader/cache/CachedCSSStyleSheet.cpp:
    (WebCore::CachedCSSStyleSheet::finishLoading):
    (WebCore::CachedCSSStyleSheet::protectedDecoder const):
    * Source/WebCore/loader/cache/CachedCSSStyleSheet.h:

    Canonical link: https://commits.webkit.org/267815.575@safari-7617-branch

Identifier: 267815.572@safari-7617.1.17.11-branch
  • Loading branch information
MyahCobbs committed Nov 16, 2023
1 parent b66ff30 commit bda88b9
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 14 deletions.
26 changes: 15 additions & 11 deletions Source/WebCore/loader/SubresourceLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -724,15 +724,19 @@ void SubresourceLoader::didFinishLoading(const NetworkLoadMetrics& networkLoadMe

if (m_state != Initialized)
return;

Ref protectedThis { *this };

ASSERT(!reachedTerminalState());
ASSERT(!m_resource->resourceToRevalidate());
// FIXME (129394): We should cancel the load when a decode error occurs instead of continuing the load to completion.
ASSERT(!m_resource->errorOccurred() || m_resource->status() == CachedResource::DecodeError || !m_resource->isLoading());
LOG(ResourceLoading, "Received '%s'.", m_resource->url().string().latin1().data());
logResourceLoaded(m_frame.get(), m_resource->type());
CachedResourceHandle resource = m_resource.get();
if (!resource)
return;

Ref<SubresourceLoader> protectedThis(*this);
CachedResourceHandle<CachedResource> protectResource(m_resource.get());
ASSERT(!resource->resourceToRevalidate());
// FIXME (129394): We should cancel the load when a decode error occurs instead of continuing the load to completion.
ASSERT(!resource->errorOccurred() || resource->status() == CachedResource::DecodeError || !resource->isLoading());
LOG(ResourceLoading, "Received '%s'.", resource->url().string().latin1().data());
logResourceLoaded(m_frame.get(), resource->type());

m_loadTiming.markEndTime();

Expand All @@ -742,22 +746,22 @@ void SubresourceLoader::didFinishLoading(const NetworkLoadMetrics& networkLoadMe
// This is the legacy path for platforms (and ResourceHandle paths) that do not provide
// complete load metrics in didFinishLoad. In those cases, fall back to the possibility
// that they populated partial load timing information on the ResourceResponse.
const auto* timing = m_resource->response().deprecatedNetworkLoadMetricsOrNull();
const auto* timing = resource->response().deprecatedNetworkLoadMetricsOrNull();
reportResourceTiming(timing ? *timing : NetworkLoadMetrics::emptyMetrics());
}

if (m_resource->type() != CachedResource::Type::MainResource)
if (resource->type() != CachedResource::Type::MainResource)
tracePoint(SubresourceLoadDidEnd, identifier().toUInt64());

m_state = Finishing;
m_resource->finishLoading(resourceData(), networkLoadMetrics);
resource->finishLoading(resourceData(), networkLoadMetrics);

if (wasCancelled()) {
SUBRESOURCELOADER_RELEASE_LOG("didFinishLoading: was canceled");
return;
}

m_resource->finish();
resource->finish();
ASSERT(!reachedTerminalState());
didFinishLoadingOnePart(networkLoadMetrics);
notifyDone(LoadCompletionType::Finish);
Expand Down
7 changes: 6 additions & 1 deletion Source/WebCore/loader/cache/CachedCSSStyleSheet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ void CachedCSSStyleSheet::finishLoading(const FragmentedSharedBuffer* data, cons
auto contiguousData = data->makeContiguous();
setEncodedSize(data->size());
// Decode the data to find out the encoding and keep the sheet text around during checkNotify()
m_decodedSheetText = m_decoder->decodeAndFlush(contiguousData->data(), data->size());
m_decodedSheetText = protectedDecoder()->decodeAndFlush(contiguousData->data(), data->size());
m_data = WTFMove(contiguousData);
} else {
m_data = nullptr;
Expand All @@ -116,6 +116,11 @@ void CachedCSSStyleSheet::finishLoading(const FragmentedSharedBuffer* data, cons
m_decodedSheetText = String();
}

Ref<TextResourceDecoder> CachedCSSStyleSheet::protectedDecoder() const
{
return m_decoder;
}

void CachedCSSStyleSheet::checkNotify(const NetworkLoadMetrics&)
{
if (isLoading())
Expand Down
5 changes: 3 additions & 2 deletions Source/WebCore/loader/cache/CachedCSSStyleSheet.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,20 +49,21 @@ class CachedCSSStyleSheet final : public CachedResource {
String responseMIMEType() const;
bool canUseSheet(MIMETypeCheckHint, bool* hasValidMIMEType) const;
bool mayTryReplaceEncodedData() const final { return true; }
Ref<TextResourceDecoder> protectedDecoder() const;

void didAddClient(CachedResourceClient&) final;

void setEncoding(const String&) final;
String encoding() const final;
const TextResourceDecoder* textResourceDecoder() const final { return m_decoder.get(); }
const TextResourceDecoder* textResourceDecoder() const final { return m_decoder.ptr(); }
void finishLoading(const FragmentedSharedBuffer*, const NetworkLoadMetrics&) final;
void destroyDecodedData() final;

void setBodyDataFrom(const CachedResource&) final;

void checkNotify(const NetworkLoadMetrics&) final;

RefPtr<TextResourceDecoder> m_decoder;
Ref<TextResourceDecoder> m_decoder;
String m_decodedSheetText;

RefPtr<StyleSheetContents> m_parsedStyleSheetCache;
Expand Down

0 comments on commit bda88b9

Please sign in to comment.