CVE-2026-7348
Overview
Background
- `ImageDecodingStore`
- Blink’s process-wide cache of
ImageDecoderobjects, keyed by generator/size/alpha, with a map plus an intrusive LRU list for eviction. - `InsertCacheInternal`
- the templated helper that records a new cache entry: it appends the entry to
ordered_cache_list_, updatesheap_memory_usage_in_bytes_, and inserts into thecache_mapandidentifier_map. - Intrusive LRU list (`ordered_cache_list_`)
- a linked list that stores raw pointers into cache entries so they can be reordered/evicted cheaply; a stale raw pointer in it dangles if the entry is freed.
- `LockDecoder` / `InsertDecoder`
- the lookup-then-insert pair where a cache miss in
LockDecoderleads a caller to decode and then callInsertDecoder, which can race across threads for the same key.
Root Cause Analysis
The old InsertCacheInternal added the raw entry pointer to ordered_cache_list_ and bumped heap_memory_usage_in_bytes_ before inserting the unique_ptr into cache_map. WTF::HashMap::insert does not overwrite an existing key, so when a duplicate key was inserted the moved-in unique_ptr was not consumed, destroyed at end of scope, and the just-freed object remained referenced by the LRU list — a classic use-after-free that any later LRU traversal (e.g. pruning) would dereference. The only release-build guard against duplicates was a DCHECK(!decoder_cache_map_.Contains(...)) in InsertDecoder, which is compiled out in production and, per the commit message, could not actually prevent duplicates because two threads can each miss in LockDecoder and both proceed to InsertDecoder for the same key.
The fix inverts the order: InsertCacheInternal attempts cache_map->insert first and, when result.is_new_entry is false, returns immediately so the redundant unique_ptr is destroyed without ever touching the LRU list or accounting. Only on a genuinely new entry does it append entry_ptr to ordered_cache_list_, add MemoryUsageInBytes(), and update the identifier_map, keeping the list and the map consistent.
!is_new_entry makes the LRU/accounting updates conditional on actual ownership transfer.Attack Path
- Force repeated decodes of one image
Web content drives decoding of the same image (same generator/size/alpha), so lookups route through
LockDecoderand misses lead toInsertDecoder. - Produce a duplicate key
Two threads concurrently miss in
LockDecoderfor the same key (neither has inserted yet), each decode, and each callInsertDecoderwith its own decoder, yielding a duplicate insertion. - Leave a dangling LRU pointer
The second
InsertCacheInternalappends the raw entry pointer toordered_cache_list_, thencache_map->insertrejects the duplicate key, the un-consumedunique_ptris destroyed, and the freed object stays linked in the LRU list. - Trigger the use-after-free
A later LRU operation such as pruning (
SetCacheLimitInBytes(0)in the regression test) walksordered_cache_list_and dereferences the stale pointer (inferred: the specific faulting access and any further primitive are not shown beyond the list dereference).
Impact Assessment
LockDecoder/InsertDecoder race (or otherwise producing a duplicate key), which is inherently timing-dependent. Any escalation beyond the dangling-pointer condition (controlled reuse, info leak, or RCE) is (inferred) and not demonstrated by the diff.Changed Functions
| Function | Change | Notes |
|---|---|---|
ifthird_party/blink/renderer/platform/graphics/image_decoding_store.cc |
modified | |
TEST_Fthird_party/blink/renderer/platform/graphics/image_decoding_store_test.cc |
modified |
Files Changed
third_party/blink/renderer/platform/graphics/image_decoding_store.ccthird_party/blink/renderer/platform/graphics/image_decoding_store_test.cc
Audit Directions
- Publish-before-commit orderingFlag any code that inserts a raw pointer into an intrusive list or global accounting structure before the owning map/container insertion that could fail or reject (e.g. no-overwrite
HashMap::insert). - No-overwrite insert semanticsAudit uses of
WTF::HashMap::insert/AddResultwhere a non-is_new_entryresult silently drops a moved-in value; confirm the dropped object is not already referenced elsewhere. - DCHECK-only uniqueness guards under concurrencyLook for caches that assume key uniqueness via
DCHECK/CHECKwhile a lookup-miss-then-insert pattern across threads (likeLockDecoder/InsertDecoder) can produce duplicates in release builds.
Patch
From b3cd8e4aa3f6c3f0fe05c7bca29d4215cddcd825 Mon Sep 17 00:00:00 2001 From: Andrew Paseltiner <[email protected]> Date: Fri, 10 Apr 2026 07:33:16 -0700 Subject: [PATCH] Fix Use-After-Free in ImageDecodingStore This CL fixes a UAF vulnerability in ImageDecodingStore where a raw pointer was added to an intrusive LRU list before map insertion. If insertion failed (e.g. due to duplicate keys), the object was freed while the dangling pointer remained in the list. The logic is now inverted to attempt map insertion first. This ensures that the LRU list and memory usage accounting are only updated if the insertion actually succeeds. If a duplicate key is encountered, the redundant object is safely destroyed without ever being added to the LRU list. MemoryUsageInBytes() and Generator() are only called on successful insertions to avoid unnecessary work for duplicate entries. This approach was chosen for its simplicity and because it guarantees internal consistency regardless of how a duplicate is triggered. Duplicate keys remain possible even with perfect decoder math due to potential race conditions. For example, multiple threads can simultaneously experience a cache miss for the same key in LockDecoder and independently proceed to create and insert their own decoders via InsertDecoder. While LockDecoder ensures that a cached decoder has only one user at a time, it does not prevent these redundant insertion attempts. Regression tests added in ImageDecodingStoreTest. Bug: 500104917 Change-Id: I5205cc5168ff2dc58be94bfe9be6b06e2d17fce5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7736761 Reviewed-by: Colin Blundell <[email protected]> Commit-Queue: Andrew Paseltiner <[email protected]> Reviewed-by: Wan-Teh Chang <[email protected]> Cr-Commit-Position: refs/heads/main@{#1612855} --- diff --git a/third_party/blink/renderer/platform/graphics/image_decoding_store.cc b/third_party/blink/renderer/platform/graphics/image_decoding_store.cc index be865654..eb6074e8 100644 --- a/third_party/blink/renderer/platform/graphics/image_decoding_store.cc +++ b/third_party/blink/renderer/platform/graphics/image_decoding_store.cc @@ -111,7 +111,9 @@ generator, 0, std::move(decoder), client_id); base::AutoLock lock(lock_); - DCHECK(!decoder_cache_map_.Contains(new_cache_entry->CacheKey())); + // Note: duplicate insertions can happen if multiple threads experience a + // cache miss for the same key and both attempt to insert a decoder. + // InsertCacheInternal handles this safely. InsertCacheInternal(std::move(new_cache_entry), &decoder_cache_map_, &decoder_cache_key_map_); } @@ -228,18 +230,27 @@ U* cache_map, V* identifier_map) { lock_.AssertAcquired(); - const size_t cache_entry_bytes = cache_entry->MemoryUsageInBytes(); - heap_memory_usage_in_bytes_ += cache_entry_bytes; + const typename U::KeyType key = cache_entry->CacheKey(); - // m_orderedCacheList is used to support LRU operations to reorder cache + // Attempt to insert into the cache map first. If the key already exists, + // the unique_ptr is not consumed and will be destroyed, which is correct + // for a duplicate entry. + auto result = cache_map->insert(key, std::move(cache_entry)); + if (!result.is_new_entry) { + return; + } + + // Only add to the LRU list and update memory usage if this is a new entry. + T* entry_ptr = result.stored_value->value.get(); + + // ordered_cache_list_ is used to support LRU operations to reorder cache // entries quickly. - ordered_cache_list_.Append(cache_entry.get()); + ordered_cache_list_.Append(entry_ptr); + heap_memory_usage_in_bytes_ += entry_ptr->MemoryUsageInBytes(); - typename U::KeyType key = cache_entry->CacheKey(); - typename V::AddResult result = identifier_map->insert( - cache_entry->Generator(), typename V::MappedType()); - result.stored_value->value.insert(key); - cache_map->insert(key, std::move(cache_entry)); + typename V::AddResult id_result = + identifier_map->insert(entry_ptr->Generator(), typename V::MappedType()); + id_result.stored_value->value.insert(key); TRACE_COUNTER1(TRACE_DISABLED_BY_DEFAULT("blink.image_decoding"), "ImageDecodingStoreHeapMemoryUsageBytes", diff --git a/third_party/blink/renderer/platform/graphics/image_decoding_store_test.cc b/third_party/blink/renderer/platform/graphics/image_decoding_store_test.cc index 9781cb9a..1881c7a 100644 --- a/third_party/blink/renderer/platform/graphics/image_decoding_store_test.cc +++ b/third_party/blink/renderer/platform/graphics/image_decoding_store_test.cc @@ -235,4 +235,68 @@ EXPECT_EQ(image_decoding_store_.CacheEntries(), 0); } +// Regression test for crbug.com/500104917. +TEST_F(ImageDecodingStoreTest, DuplicateInsert) { + auto decoder1 = std::make_unique<MockImageDecoder>(this); + decoder1->SetSize(1, 1); + image_decoding_store_.InsertDecoder(generator_.get(), + cc::PaintImage::kDefaultGeneratorClientId, + std::move(decoder1)); + EXPECT_EQ(1, image_decoding_store_.CacheEntries()); + + auto decoder2 = std::make_unique<MockImageDecoder>(this); + decoder2->SetSize(1, 1); + + // Duplicate insertion should be handled safely. + image_decoding_store_.InsertDecoder(generator_.get(), + cc::PaintImage::kDefaultGeneratorClientId, + std::move(decoder2)); + + // Should still have only 1 entry. + EXPECT_EQ(1, image_decoding_store_.CacheEntries()); + + // Pruning should work correctly and not crash. + image_decoding_store_.SetCacheLimitInBytes(0); + EXPECT_EQ(0, image_decoding_store_.CacheEntries()); +} + +// Regression test for crbug.com/500104917. +// Simulates a race condition where multiple threads experience a cache miss +// for the same key and both attempt to insert a decoder. +TEST_F(ImageDecodingStoreTest, LockDecoderMissRace) { + const SkISize size = SkISize::Make(1, 1); + const ImageDecoder::AlphaOption alpha = ImageDecoder::kAlphaPremultiplied; + const cc::PaintImage::GeneratorClientId client_id = + cc::PaintImage::kDefaultGeneratorClientId; + + ImageDecoder* decoder; + // Thread 1: experiences a cache miss. + EXPECT_FALSE(image_decoding_store_.LockDecoder(generator_.get(), size, alpha, + client_id, &decoder)); + + // Thread 2: also experiences a cache miss for the same key because Thread 1 + // hasn't called InsertDecoder() yet. + EXPECT_FALSE(image_decoding_store_.LockDecoder(generator_.get(), size, alpha, + client_id, &decoder)); + + // Thread 1: completes decoding and inserts the decoder. + auto decoder1 = std::make_unique<MockImageDecoder>(this); + decoder1->SetSize(1, 1); + image_decoding_store_.InsertDecoder(generator_.get(), client_id, + std::move(decoder1)); + EXPECT_EQ(1, image_decoding_store_.CacheEntries()); + + // Thread 2: also completes decoding and attempts to insert its own decoder + // for the same key. This is a duplicate insertion. + auto decoder2 = std::make_unique<MockImageDecoder>(this); + decoder2->SetSize(1, 1); + image_decoding_store_.InsertDecoder(generator_.get(), client_id, + std::move(decoder2)); + + // The store should handle the duplicate safely and maintain consistency. + EXPECT_EQ(1, image_decoding_store_.CacheEntries()); + image_decoding_store_.SetCacheLimitInBytes(0); + EXPECT_EQ(0, image_decoding_store_.CacheEntries()); +} + } // namespace blink
Regression Test / PoC
diff --git a/third_party/blink/renderer/platform/graphics/image_decoding_store_test.cc b/third_party/blink/renderer/platform/graphics/image_decoding_store_test.cc
index 9781cb9a..1881c7a 100644
--- a/third_party/blink/renderer/platform/graphics/image_decoding_store_test.cc
+++ b/third_party/blink/renderer/platform/graphics/image_decoding_store_test.cc
@@ -235,4 +235,68 @@
EXPECT_EQ(image_decoding_store_.CacheEntries(), 0);
}
+// Regression test for crbug.com/500104917.
+TEST_F(ImageDecodingStoreTest, DuplicateInsert) {
+ auto decoder1 = std::make_unique<MockImageDecoder>(this);
+ decoder1->SetSize(1, 1);
+ image_decoding_store_.InsertDecoder(generator_.get(),
+ cc::PaintImage::kDefaultGeneratorClientId,
+ std::move(decoder1));
+ EXPECT_EQ(1, image_decoding_store_.CacheEntries());
+
+ auto decoder2 = std::make_unique<MockImageDecoder>(this);
+ decoder2->SetSize(1, 1);
+
+ // Duplicate insertion should be handled safely.
+ image_decoding_store_.InsertDecoder(generator_.get(),
+ cc::PaintImage::kDefaultGeneratorClientId,
+ std::move(decoder2));
+
+ // Should still have only 1 entry.
+ EXPECT_EQ(1, image_decoding_store_.CacheEntries());
+
+ // Pruning should work correctly and not crash.
+ image_decoding_store_.SetCacheLimitInBytes(0);
+ EXPECT_EQ(0, image_decoding_store_.CacheEntries());
+}
+
+// Regression test for crbug.com/500104917.
+// Simulates a race condition where multiple threads experience a cache miss
+// for the same key and both attempt to insert a decoder.
+TEST_F(ImageDecodingStoreTest, LockDecoderMissRace) {
+ const SkISize size = SkISize::Make(1, 1);
+ const ImageDecoder::AlphaOption alpha = ImageDecoder::kAlphaPremultiplied;
+ const cc::PaintImage::GeneratorClientId client_id =
+ cc::PaintImage::kDefaultGeneratorClientId;
+
+ ImageDecoder* decoder;
+ // Thread 1: experiences a cache miss.
+ EXPECT_FALSE(image_decoding_store_.LockDecoder(generator_.get(), size, alpha,
+ client_id, &decoder));
+
+ // Thread 2: also experiences a cache miss for the same key because Thread 1
+ // hasn't called InsertDecoder() yet.
+ EXPECT_FALSE(image_decoding_store_.LockDecoder(generator_.get(), size, alpha,
+ client_id, &decoder));
+
+ // Thread 1: completes decoding and inserts the decoder.
+ auto decoder1 = std::make_unique<MockImageDecoder>(this);
+ decoder1->SetSize(1, 1);
+ image_decoding_store_.InsertDecoder(generator_.get(), client_id,
+ std::move(decoder1));
+ EXPECT_EQ(1, image_decoding_store_.CacheEntries());
+
+ // Thread 2: also completes decoding and attempts to insert its own decoder
+ // for the same key. This is a duplicate insertion.
+ auto decoder2 = std::make_unique<MockImageDecoder>(this);
+ decoder2->SetSize(1, 1);
+ image_decoding_store_.InsertDecoder(generator_.get(), client_id,
+ std::move(decoder2));
+
+ // The store should handle the duplicate safely and maintain consistency.
+ EXPECT_EQ(1, image_decoding_store_.CacheEntries());
+ image_decoding_store_.SetCacheLimitInBytes(0);
+ EXPECT_EQ(0, image_decoding_store_.CacheEntries());
+}
+
} // namespace blink
Original Bug Report
Use-After-Free in ImageDecodingStore via Duplicate Key Insertion
Project Fortify, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports without the security team.
Overview: A Use-After-Free (UAF) vulnerability exists in ImageDecodingStore due to a raw pointer being appended to an intrusive list before a hash map insertion. If the insertion fails due to a duplicate key, the object is destroyed but the dangling pointer remains in the list. This can be triggered by a float precision flaw during progressive JPEG decoding.
Affected files:
third_party/blink/renderer/platform/graphics/image_decoding_store.ccthird_party/blink/renderer/platform/graphics/image_decoding_store.hthird_party/blink/renderer/platform/image-decoders/jpeg/jpeg_image_decoder.cc
Estimated timestamp from git blame: 2024-06-29
Summary
A highly exploitable Use-After-Free (UAF) vulnerability exists in the Blink rendering engine’s ImageDecodingStore. The issue occurs because a raw pointer to a newly created DecoderCacheEntry is appended to an intrusive LRU list (ordered_cache_list_) before the entry is inserted into a hash map (decoder_cache_map_). If the hash map insertion fails due to a duplicate key, the std::unique_ptr holding the entry is destroyed, but the raw pointer remains in the intrusive list.
Because the list pointers in CacheEntry are explicitly annotated with RAW_PTR_EXCLUSION, this memory is not protected by MiraclePtr (BackupRefPtr). Subsequent cache pruning operations will perform virtual method calls on the freed object, potentially leading to Remote Code Execution (RCE) in the sandboxed renderer process.
Vulnerability Details
There are two components to this vulnerability: the trigger and the UAF logic flaw.
1. The Trigger (Math Precision Loss):
In JPEGImageDecoder::DesiredScaleNumerator, an image downscale factor is calculated using a single-precision float cast ((float)max_decoded_bytes). For very large image dimensions, this cast loses precision. Consequently, a request to scale an image to a specific size (e.g., 4101x4101) can result in an actual DecodedSize() that slightly differs (e.g., 2734x2734).
When ImageDecoderWrapper::Decode attempts to find a cached decoder, it searches using the requested size (LockDecoder(4101x4101)). When it fails to find one, it decodes the image and caches the new decoder using its actual size (InsertDecoder(2734x2734)). On subsequent decode passes of the same progressive JPEG, LockDecoder misses again, causing a second attempt to insert a decoder with the same 2734x2734 key.
2. The UAF Flaw (InsertCacheInternal):
In third_party/blink/renderer/platform/graphics/image_decoding_store.cc, the InsertCacheInternal method performs the following sequence:
// 1. Appends raw pointer to the list.
ordered_cache_list_.Append(cache_entry.get());
// ...
// 2. Attempts map insertion.
cache_map->insert(key, std::move(cache_entry));
If the key already exists (as in the duplicate trigger described above), WTF::HashMap::insert returns without consuming the unique_ptr. The function returns, the unique_ptr goes out of scope, and the memory is freed. However, the raw pointer remains in ordered_cache_list_. Furthermore, heap_memory_usage_in_bytes_ is incremented prior to the insertion but never decremented on failure, causing a memory leak that can artificially force the cache to permanently prune on every subsequent decode.
Potential Exploit Scenario
Note: These are potential steps based on code analysis; our tooling does not yet execute live exploit code.
- Trigger: An attacker hosts a malicious webpage serving a progressive JPEG with extremely large dimensions, styled with CSS to a specific scale that triggers the
floatprecision loss. - Duplicate Insertion: The browser performs an initial decode of the first JPEG scan and caches the decoder. As the next scan arrives, the math mismatch causes a cache miss, resulting in a second cache insertion attempt for the same key.
- UAF Creation: The duplicate insertion fails. The second
DecoderCacheEntryis freed, leaving a dangling pointer in theordered_cache_list_intrusive list. - Heap Grooming: The attacker uses JavaScript to allocate objects of similar size in the FastMalloc partition, reclaiming the freed
DecoderCacheEntrymemory block. The attacker populates this block with controlled data, setting theuse_count_(at offset0x8) to0and pointing the vtable pointer (at offset0x0) to a controlled ROP chain. - Execution: The attacker triggers one final image decode, or the memory leak forces a
Prune(). ThePrune()method iterates over the list, encounters the dangling pointer, passes the!UseCount()check, and callsRemoveFromCacheInternal. This immediately invokes the virtual methodcache_entry->GetType(), hijacking control flow to the attacker’s payload.
Suggested Fix
- Fix the UAF in
InsertCacheInternal: The logic should be inverted. Attempt thecache_map->insert()operation first. If it succeeds (i.e.,is_new_entryis true), then append the raw pointer toordered_cache_list_and incrementheap_memory_usage_in_bytes_. - Fix the Math Precision: In
JPEGImageDecoder::DesiredScaleNumerator, usedoubleinstead offloatfor intermediate calculations involvingwtf_size_t(which is a 32-bit integer) to prevent precision loss when calculating dimensions for large images.
Evaluated with Chrome root at commit: f200f57a19490707ff8bc7aa5de3cbc443a3afad
Results so far have been promising, but there can be wrong deductions. If this proves to be a false positive, please close as WAI; data from false positives will be used to improve accuracy over time. And please feel free to reach out to me directly if you have concerns or feedback on the project.