[Backport] CVE-2021-21209: Inappropriate implementation in storage (2/5)

Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/2648212:
CacheStorage: Make LegacyCacheStorage::SizeImpl respect padding.

Previously LegacyCacheStorage::SizeImpl() would include the full padded
size of a Cache object, but it would not detect if the padding had been
invalidated for some reason.  In addition, it did not properly propagate
the size information to doomed caches.  This CL corrects those issues.

Note, this CL does not contain a test.  A follow-up CL that performs
a padding migration will include a test that exercises this path.  For
now this CL has been manually tested and verified.  This CL was split
out from the migration CL in an attempt to reduce CL size and make them
easier to understand.

Bug: 1143526
Change-Id: I049adbe4a5cc931dc079f330ffa27f9212eb2fa7
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/master@{#847262}
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

2 files changed, 14 insertions, 7 deletions

diff --git a/chromium/content/browser/cache_storage/legacy/legacy_cache_storage.cc b/chromium/content/browser/cache_storage/legacy/legacy_cache_storage.cc
index 6915a3b34cf..28a928b65f6 100644
--- a/chromium/content/browser/cache_storage/legacy/legacy_cache_storage.cc
+++ b/chromium/content/browser/cache_storage/legacy/legacy_cache_storage.cc
@@ -1380,9 +1380,11 @@ void LegacyCacheStorage::SizeRetrievedFromCache(
     int64_t* accumulator,
     int64_t size) {
   auto* impl = LegacyCacheStorageCache::From(cache_handle);
-  if (doomed_caches_.find(impl) == doomed_caches_.end())
-    cache_index_->SetCacheSize(impl->cache_name(), size);
-  *accumulator += size;
+  if (doomed_caches_.find(impl) == doomed_caches_.end()) {
+    cache_index_->SetCacheSize(impl->cache_name(), impl->cache_size());
+    cache_index_->SetCachePadding(impl->cache_name(), impl->cache_padding());
+  }
+  *accumulator += (impl->cache_size() + impl->cache_padding());
   std::move(closure).Run();
 }
 
@@ -1435,8 +1437,9 @@ void LegacyCacheStorage::SizeImpl(SizeCallback callback) {
                      std::move(callback)));
 
   for (const auto& cache_metadata : cache_index_->ordered_cache_metadata()) {
-    if (cache_metadata.size != LegacyCacheStorage::kSizeUnknown) {
-      *accumulator_ptr += cache_metadata.size;
+    if (cache_metadata.size != LegacyCacheStorage::kSizeUnknown &&
+        cache_metadata.padding != LegacyCacheStorage::kSizeUnknown) {
+      *accumulator_ptr += (cache_metadata.size + cache_metadata.padding);
       barrier_closure.Run();
       continue;
     }
diff --git a/chromium/content/browser/cache_storage/legacy/legacy_cache_storage_cache.h b/chromium/content/browser/cache_storage/legacy/legacy_cache_storage_cache.h
index a2b541a5293..099b9c49e36 100644
--- a/chromium/content/browser/cache_storage/legacy/legacy_cache_storage_cache.h
+++ b/chromium/content/browser/cache_storage/legacy/legacy_cache_storage_cache.h
@@ -147,11 +147,15 @@ class CONTENT_EXPORT LegacyCacheStorageCache : public CacheStorageCache {
   // will exit early. Close should only be called once per CacheStorageCache.
   void Close(base::OnceClosure callback);
 
-  // The size of the cache's contents.
+  // The size of the cache's contents.  The callback reports the padded
+  // size.  If you want the unpadded size you may call the cache_size()
+  // getter method on the cache object when the callback is invoked; the
+  // getter will have an up-to-date value at that point.
   void Size(SizeCallback callback);
 
   // Gets the cache's size, closes the backend, and then runs |callback| with
-  // the cache's size.
+  // the cache's size.  As per the comment for Size(), this also returns the
+  // padded size.
   void GetSizeThenClose(SizeCallback callback);
 
   void Put(blink::mojom::FetchAPIRequestPtr request,