Skip to content

Commit d38fbb1

Browse files
yoshisatoyanagisawamibrunin
authored andcommitted
[Backport] CVE-2024-11110: Inappropriate implementation in Blink
Manual backport of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/5961018: Make GetCacheIdentifier() respect GetSkipServiceWorker(). M126 merge issues: third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc: RequestResource()/CreateResourceForLoading(): The kScopeMemoryCachePerContext feature check isn't present in the original CL. Since the current GetCacheIdentifier() ignores GetSkipServiceWorker(), GetCacheIdentifier() returns ServiceWorkerId even if GetSkipServiceWorker() is true if the ServiceWorker has a fetch handler. To make the isolated world respected as an isolated world, the cache identifier should not be shared with a page under a ServiceWorker control. Bug: 372512079, 373263969 Change-Id: Idd2d8900f2f720e0a4dc9837e2eb56474c60b587 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5961018 Commit-Queue: Yoshisato Yanagisawa <[email protected]> Cr-Commit-Position: refs/heads/main@{#1376006} (cherry picked from commit 75f322ad1f64c0bc56fa77ab877b48d72cdb903c) Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/607609 Reviewed-by: Anu Aliyas <[email protected]>
1 parent 328f848 commit d38fbb1

File tree

7 files changed

+29
-15
lines changed

7 files changed

+29
-15
lines changed

chromium/third_party/blink/renderer/core/html/parser/html_srcset_parser.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,7 @@ static unsigned AvoidDownloadIfHigherDensityResourceIsInCache(
413413
KURL url = document->CompleteURL(
414414
StripLeadingAndTrailingHTMLSpaces(image_candidates[i]->Url()));
415415
if (MemoryCache::Get()->ResourceForURL(
416-
url, document->Fetcher()->GetCacheIdentifier(url)) ||
416+
url, document->Fetcher()->GetCacheIdentifier(url, /*skip_service_worker=*/false)) ||
417417
url.ProtocolIsData())
418418
return i;
419419
}

chromium/third_party/blink/renderer/core/inspector/inspector_network_agent.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2315,7 +2315,8 @@ bool InspectorNetworkAgent::FetchResourceContent(Document* document,
23152315
Resource* cached_resource = document->Fetcher()->CachedResource(url);
23162316
if (!cached_resource) {
23172317
cached_resource = MemoryCache::Get()->ResourceForURL(
2318-
url, document->Fetcher()->GetCacheIdentifier(url));
2318+
url, document->Fetcher()->GetCacheIdentifier(
2319+
url, /*skip_service_worker=*/false));
23192320
}
23202321
if (cached_resource && InspectorPageAgent::CachedResourceContent(
23212322
cached_resource, content, base64_encoded)) {

chromium/third_party/blink/renderer/core/inspector/inspector_page_agent.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,8 @@ Resource* CachedResource(LocalFrame* frame,
167167
Resource* cached_resource = document->Fetcher()->CachedResource(url);
168168
if (!cached_resource) {
169169
cached_resource = MemoryCache::Get()->ResourceForURL(
170-
url, document->Fetcher()->GetCacheIdentifier(url));
170+
url, document->Fetcher()->GetCacheIdentifier(
171+
url, /*skip_service_worker=*/false));
171172
}
172173
if (!cached_resource)
173174
cached_resource = loader->ResourceForURL(url);

chromium/third_party/blink/renderer/core/loader/image_loader.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -741,7 +741,8 @@ bool ImageLoader::ShouldLoadImmediately(const KURL& url) const {
741741
// content when style recalc is over and DOM mutation is allowed again.
742742
if (!url.IsNull()) {
743743
Resource* resource = MemoryCache::Get()->ResourceForURL(
744-
url, element_->GetDocument().Fetcher()->GetCacheIdentifier(url));
744+
url, element_->GetDocument().Fetcher()->GetCacheIdentifier(
745+
url, /*skip_service_worker=*/false));
745746

746747
if (resource && !resource->ErrorOccurred() &&
747748
CanReuseFromListOfAvailableImages(

chromium/third_party/blink/renderer/core/testing/internals.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -883,8 +883,8 @@ bool Internals::isLoading(const String& url) {
883883
if (!document_)
884884
return false;
885885
const KURL full_url = document_->CompleteURL(url);
886-
const String cache_identifier =
887-
document_->Fetcher()->GetCacheIdentifier(full_url);
886+
const String cache_identifier = document_->Fetcher()->GetCacheIdentifier(
887+
full_url, /*skip_service_worker=*/false);
888888
Resource* resource =
889889
MemoryCache::Get()->ResourceForURL(full_url, cache_identifier);
890890
// We check loader() here instead of isLoading(), because a multipart
@@ -896,8 +896,8 @@ bool Internals::isLoadingFromMemoryCache(const String& url) {
896896
if (!document_)
897897
return false;
898898
const KURL full_url = document_->CompleteURL(url);
899-
const String cache_identifier =
900-
document_->Fetcher()->GetCacheIdentifier(full_url);
899+
const String cache_identifier = document_->Fetcher()->GetCacheIdentifier(
900+
full_url, /*skip_service_worker=*/false);
901901
Resource* resource =
902902
MemoryCache::Get()->ResourceForURL(full_url, cache_identifier);
903903
return resource && resource->GetStatus() == ResourceStatus::kCached;

chromium/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -860,7 +860,8 @@ Resource* ResourceFetcher::CreateResourceForStaticData(
860860
if (!archive_ && factory.GetType() == ResourceType::kRaw)
861861
return nullptr;
862862

863-
const String cache_identifier = GetCacheIdentifier(url);
863+
const String cache_identifier = GetCacheIdentifier(
864+
url, params.GetResourceRequest().GetSkipServiceWorker());
864865
// Most off-main-thread resource fetches use Resource::kRaw and don't reach
865866
// this point, but off-main-thread module fetches might.
866867
if (IsMainThread()) {
@@ -1347,7 +1348,10 @@ Resource* ResourceFetcher::RequestResource(FetchParameters& params,
13471348
resource = nullptr;
13481349
} else {
13491350
resource = MemoryCache::Get()->ResourceForURL(
1350-
params.Url(), GetCacheIdentifier(params.Url()));
1351+
params.Url(),
1352+
GetCacheIdentifier(
1353+
params.Url(),
1354+
params.GetResourceRequest().GetSkipServiceWorker()));
13511355
}
13521356
if (resource) {
13531357
policy = DetermineRevalidationPolicy(resource_type, params, *resource,
@@ -1604,7 +1608,8 @@ Resource* ResourceFetcher::CreateResourceForLoading(
16041608
const FetchParameters& params,
16051609
const ResourceFactory& factory) {
16061610
const String cache_identifier =
1607-
GetCacheIdentifier(params.GetResourceRequest().Url());
1611+
GetCacheIdentifier(params.GetResourceRequest().Url(),
1612+
params.GetResourceRequest().GetSkipServiceWorker());
16081613
if (!base::FeatureList::IsEnabled(
16091614
blink::features::kScopeMemoryCachePerContext)) {
16101615
DCHECK(!IsMainThread() || params.IsStaleRevalidation() ||
@@ -2605,9 +2610,11 @@ void ResourceFetcher::UpdateAllImageResourcePriorities() {
26052610
to_be_removed.clear();
26062611
}
26072612

2608-
String ResourceFetcher::GetCacheIdentifier(const KURL& url) const {
2609-
if (properties_->GetControllerServiceWorkerMode() !=
2610-
mojom::ControllerServiceWorkerMode::kNoController) {
2613+
String ResourceFetcher::GetCacheIdentifier(const KURL& url,
2614+
bool skip_service_worker) const {
2615+
if (!skip_service_worker &&
2616+
properties_->GetControllerServiceWorkerMode() !=
2617+
mojom::ControllerServiceWorkerMode::kNoController) {
26112618
return String::Number(properties_->ServiceWorkerId());
26122619
}
26132620

chromium/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,11 @@ class PLATFORM_EXPORT ResourceFetcher
260260
uint32_t inflight_keepalive_bytes);
261261
blink::mojom::ControllerServiceWorkerMode IsControlledByServiceWorker() const;
262262

263-
String GetCacheIdentifier(const KURL& url) const;
263+
// Returns a cache identifier for MemoryCache.
264+
// `url` is used for finding a matching WebBundle.
265+
// If `skip_service_worker` is true, the identifier won't be a ServiceWorker's
266+
// identifier to keep the cache separated.
267+
String GetCacheIdentifier(const KURL& url, bool skip_service_worker) const;
264268

265269
// If `url` exists as a resource in a subresource bundle in this frame,
266270
// returns its UnguessableToken; otherwise, returns absl::nullopt.

0 commit comments

Comments
 (0)