commit 36eff9701c92cc4f745dc05b6c3749d0a1e96282
parent 7f2f0b31b0ece76aa779794b1912ac8b41eaaada
Author: Randell Jesup <rjesup@mozilla.com>
Date: Mon, 15 Dec 2025 12:32:06 +0000
Bug 2005909: Validate CompressionDictionary hashes and remove bad entries on failure r=necko-reviewers,valentin
This also fails the request at an earlier point; it would (probably) failed later on decompression.
It adds new testing APIs to allow this to be easily tested.
Differential Revision: https://phabricator.services.mozilla.com/D276397
Diffstat:
6 files changed, 294 insertions(+), 17 deletions(-)
diff --git a/netwerk/cache2/CacheStorageService.cpp b/netwerk/cache2/CacheStorageService.cpp
@@ -2468,4 +2468,26 @@ CacheStorageService::ClearDictionaryCacheMemory() {
return NS_OK;
}
+NS_IMETHODIMP
+CacheStorageService::CorruptDictionaryHash(const nsACString& aURI) {
+ LOG(("CacheStorageService::CorruptDictionaryHash [uri=%s]",
+ PromiseFlatCString(aURI).get()));
+ RefPtr<DictionaryCache> cache = DictionaryCache::GetInstance();
+ if (cache) {
+ cache->CorruptHashForTesting(aURI);
+ }
+ return NS_OK;
+}
+
+NS_IMETHODIMP
+CacheStorageService::ClearDictionaryDataForTesting(const nsACString& aURI) {
+ LOG(("CacheStorageService::ClearDictionaryDataForTesting [uri=%s]",
+ PromiseFlatCString(aURI).get()));
+ RefPtr<DictionaryCache> cache = DictionaryCache::GetInstance();
+ if (cache) {
+ cache->ClearDictionaryDataForTesting(aURI);
+ }
+ return NS_OK;
+}
+
} // namespace mozilla::net
diff --git a/netwerk/cache2/Dictionary.cpp b/netwerk/cache2/Dictionary.cpp
@@ -215,9 +215,9 @@ void DictionaryCacheEntry::UseCompleted() {
}
// returns aShouldSuspend=true if we should suspend to wait for the prefetch
-nsresult DictionaryCacheEntry::Prefetch(nsILoadContextInfo* aLoadContextInfo,
- bool& aShouldSuspend,
- const std::function<void()>& aFunc) {
+nsresult DictionaryCacheEntry::Prefetch(
+ nsILoadContextInfo* aLoadContextInfo, bool& aShouldSuspend,
+ const std::function<void(nsresult)>& aFunc) {
DICTIONARY_LOG(("Prefetch for %s", mURI.get()));
// Start reading the cache entry into memory and call completion
// function when done
@@ -532,19 +532,43 @@ DictionaryCacheEntry::OnStopRequest(nsIRequest* request, nsresult result) {
DICTIONARY_LOG(("DictionaryCacheEntry %s OnStopRequest", mURI.get()));
if (NS_SUCCEEDED(result)) {
mDictionaryDataComplete = true;
+
+ // Validate that the loaded dictionary data matches the stored hash
+ if (!mHash.IsEmpty()) {
+ nsCOMPtr<nsICryptoHash> hasher =
+ do_CreateInstance(NS_CRYPTO_HASH_CONTRACTID);
+ if (hasher) {
+ hasher->Init(nsICryptoHash::SHA256);
+ hasher->Update(mDictionaryData.begin(),
+ static_cast<uint32_t>(mDictionaryData.length()));
+ nsAutoCString computedHash;
+ MOZ_ALWAYS_SUCCEEDS(hasher->Finish(true, computedHash));
+
+ if (!computedHash.Equals(mHash)) {
+ DICTIONARY_LOG(("Hash mismatch for %s: expected %s, computed %s",
+ mURI.get(), mHash.get(), computedHash.get()));
+ result = NS_ERROR_CORRUPTED_CONTENT;
+ mDictionaryDataComplete = false;
+ mDictionaryData.clear();
+ // Remove this corrupted dictionary entry
+ DictionaryCache::RemoveDictionaryFor(mURI);
+ }
+ }
+ }
+
DICTIONARY_LOG(("Unsuspending %zu channels, Dictionary len %zu",
mWaitingPrefetch.Length(), mDictionaryData.length()));
// if we suspended, un-suspend the channel(s)
for (auto& lambda : mWaitingPrefetch) {
- (lambda)();
+ (lambda)(result);
}
mWaitingPrefetch.Clear();
} else {
- // XXX
- // This is problematic - we requested with dcb/dcz, but can't actually
- // decode them. Probably we should re-request without dcb/dcz, and also nuke
- // the entry
- // XXX
+ // Pass error to waiting callbacks
+ for (auto& lambda : mWaitingPrefetch) {
+ (lambda)(result);
+ }
+ mWaitingPrefetch.Clear();
}
// If we're being replaced by a new entry, swap now
@@ -912,6 +936,60 @@ void DictionaryCache::Clear() {
mDictionaryCache.Clear();
}
+void DictionaryCache::CorruptHashForTesting(const nsACString& aURI) {
+ DICTIONARY_LOG(("DictionaryCache::CorruptHashForTesting for %s",
+ PromiseFlatCString(aURI).get()));
+ nsCOMPtr<nsIURI> uri;
+ if (NS_FAILED(NS_NewURI(getter_AddRefs(uri), aURI))) {
+ return;
+ }
+ nsAutoCString prepath;
+ if (NS_FAILED(GetDictPath(uri, prepath))) {
+ return;
+ }
+ if (auto origin = mDictionaryCache.Lookup(prepath)) {
+ for (auto& entry : origin.Data()->mEntries) {
+ if (entry->GetURI().Equals(aURI)) {
+ DICTIONARY_LOG(("Corrupting hash for %s",
+ PromiseFlatCString(entry->GetURI()).get()));
+ entry->SetHash("CORRUPTED_FOR_TESTING"_ns);
+ return;
+ }
+ }
+ for (auto& entry : origin.Data()->mPendingEntries) {
+ if (entry->GetURI().Equals(aURI)) {
+ DICTIONARY_LOG(("Corrupting hash for %s (pending)",
+ PromiseFlatCString(entry->GetURI()).get()));
+ entry->SetHash("CORRUPTED_FOR_TESTING"_ns);
+ return;
+ }
+ }
+ }
+}
+
+void DictionaryCache::ClearDictionaryDataForTesting(const nsACString& aURI) {
+ DICTIONARY_LOG(("DictionaryCache::ClearDictionaryDataForTesting for %s",
+ PromiseFlatCString(aURI).get()));
+ nsCOMPtr<nsIURI> uri;
+ if (NS_FAILED(NS_NewURI(getter_AddRefs(uri), aURI))) {
+ return;
+ }
+ nsAutoCString prepath;
+ if (NS_FAILED(GetDictPath(uri, prepath))) {
+ return;
+ }
+ if (auto origin = mDictionaryCache.Lookup(prepath)) {
+ for (auto& entry : origin.Data()->mEntries) {
+ if (entry->GetURI().Equals(aURI)) {
+ DICTIONARY_LOG(("Clearing data for %s",
+ PromiseFlatCString(entry->GetURI()).get()));
+ entry->ClearDataForTesting();
+ return;
+ }
+ }
+ }
+}
+
// Remove a dictionary if it exists for the key given
// static
void DictionaryCache::RemoveDictionaryFor(const nsACString& aKey) {
diff --git a/netwerk/cache2/Dictionary.h b/netwerk/cache2/Dictionary.h
@@ -78,7 +78,7 @@ class DictionaryCacheEntry final : public nsICacheEntryOpenCallback,
// Start reading the cache entry into memory and call completion
// function when done
nsresult Prefetch(nsILoadContextInfo* aLoadContextInfo, bool& aShouldSuspend,
- const std::function<void()>& aFunc);
+ const std::function<void(nsresult)>& aFunc);
const nsCString& GetHash() const { return mHash; }
@@ -117,7 +117,7 @@ class DictionaryCacheEntry final : public nsICacheEntryOpenCallback,
// aFunc is called when we have finished reading a dictionary from the
// cache, or we have no users waiting for cache data (cancelled, etc)
- void CallbackOnCacheRead(const std::function<void()>& aFunc) {
+ void CallbackOnCacheRead(const std::function<void(nsresult)>& aFunc) {
// the reasons to call back are identical to Prefetch()
mWaitingPrefetch.AppendElement(aFunc);
}
@@ -126,6 +126,13 @@ class DictionaryCacheEntry final : public nsICacheEntryOpenCallback,
const Vector<uint8_t>& GetDictionary() const { return mDictionaryData; }
+ // Clear dictionary data for testing (forces reload from cache on next
+ // prefetch)
+ void ClearDataForTesting() {
+ mDictionaryData.clear();
+ mDictionaryDataComplete = false;
+ }
+
// Accumulate a hash while saving a file being received to the cache
void AccumulateHash(const char* aBuf, int32_t aCount);
void FinishHash();
@@ -193,7 +200,7 @@ class DictionaryCacheEntry final : public nsICacheEntryOpenCallback,
nsCOMPtr<nsICryptoHash> mCrypto;
// call these when prefetch is complete
- nsTArray<std::function<void()>> mWaitingPrefetch;
+ nsTArray<std::function<void(nsresult)>> mWaitingPrefetch;
// If we need to Write() an entry before we know the hash, remember the origin
// here (creates a temporary cycle). Clear on StopRequest
@@ -350,6 +357,12 @@ class DictionaryCache final {
// Clears all ports at host
void Clear();
+ // Corrupt the hash of a dictionary entry for testing
+ void CorruptHashForTesting(const nsACString& aURI);
+
+ // Clear the dictionary data while keeping the entry (for testing)
+ void ClearDictionaryDataForTesting(const nsACString& aURI);
+
// return an entry
void GetDictionaryFor(
nsIURI* aURI, ExtContentPolicyType aType, nsHttpChannel* aChan,
diff --git a/netwerk/cache2/nsICacheTesting.idl b/netwerk/cache2/nsICacheTesting.idl
@@ -22,4 +22,18 @@ interface nsICacheTesting : nsISupports
* This forces dictionary entries to be reloaded from disk on next access.
*/
void clearDictionaryCacheMemory();
+
+ /**
+ * Corrupt the hash of a stored dictionary for testing hash mismatch handling.
+ * @param aURI The URI of the dictionary to corrupt
+ */
+ void corruptDictionaryHash(in ACString aURI);
+
+ /**
+ * Clear the loaded dictionary data for a specific entry while keeping the
+ * entry metadata (including hash). This forces the data to be reloaded from
+ * disk on next prefetch.
+ * @param aURI The URI of the dictionary
+ */
+ void clearDictionaryDataForTesting(in ACString aURI);
};
diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -674,9 +674,29 @@ nsresult nsHttpChannel::PrepareToConnect() {
// weird cases like no storage service.
return NS_SUCCEEDED(self->mDictDecompress->Prefetch(
GetLoadContextInfo(self), self->mShouldSuspendForDictionary,
- [self]() {
+ [self](nsresult aResult) {
// this is called when the prefetch is complete to
// un-Suspend the channel
+ PROFILER_MARKER("Dictionary Prefetch", NETWORK,
+ MarkerTiming::IntervalEnd(), FlowMarker,
+ Flow::FromPointer(self));
+ if (NS_FAILED(aResult)) {
+ LOG(
+ ("nsHttpChannel::SetupChannelForTransaction [this=%p] "
+ "Dictionary prefetch failed: 0x%08" PRIx32,
+ self.get(), static_cast<uint32_t>(aResult)));
+ if (self->mUsingDictionary) {
+ self->mDictDecompress->UseCompleted();
+ self->mUsingDictionary = false;
+ }
+ self->mDictDecompress = nullptr;
+ if (self->mSuspendedForDictionary) {
+ self->mSuspendedForDictionary = false;
+ self->Cancel(aResult);
+ self->Resume();
+ }
+ return;
+ }
MOZ_ASSERT(self->mDictDecompress->DictionaryReady());
if (self->mSuspendedForDictionary) {
LOG(
@@ -687,9 +707,6 @@ nsresult nsHttpChannel::PrepareToConnect() {
self->mSuspendedForDictionary = false;
self->Resume();
}
- PROFILER_MARKER("Dictionary Prefetch", NETWORK,
- MarkerTiming::IntervalEnd(), FlowMarker,
- Flow::FromPointer(self));
}));
}
return true;
@@ -6298,7 +6315,7 @@ bool nsHttpChannel::ParseDictionary(nsICacheEntry* aEntry,
if (mDictSaving->ShouldSuspendUntilCacheRead()) {
LOG_DICTIONARIES(("Suspending %p to wait for cache read", this));
mTransactionPump->Suspend();
- mDictSaving->CallbackOnCacheRead([self = RefPtr(this)]() {
+ mDictSaving->CallbackOnCacheRead([self = RefPtr(this)](nsresult) {
LOG_DICTIONARIES(("Resuming %p after cache read", self.get()));
self->Resume();
});
diff --git a/netwerk/test/unit/test_dictionary_replacement.js b/netwerk/test/unit/test_dictionary_replacement.js
@@ -517,6 +517,139 @@ add_task(async function test_dictionary_multiple_replacements() {
dump("**** Test passed: Multiple replacements work correctly\n");
});
+/**
+ * Test that hash mismatch during dictionary load causes the request to fail
+ * and the corrupted dictionary entry to be removed.
+ *
+ * Steps:
+ * 1. Load a resource with Use-As-Dictionary header (creates dictionary entry)
+ * 2. Verify Available-Dictionary is sent for matching resources
+ * 3. Corrupt the hash using the testing API
+ * 4. Clear memory cache to force reload from disk
+ * 5. Request a matching resource - dictionary prefetch should fail
+ * 6. Verify the dictionary entry was removed (Available-Dictionary no longer sent)
+ */
+add_task(async function test_dictionary_hash_mismatch() {
+ dump("**** Clear cache and start fresh\n");
+ let lci = Services.loadContextInfo.custom(false, {
+ partitionKey: `(https,localhost)`,
+ });
+ evict_cache_entries("all", lci);
+
+ let testingInterface = Services.cache2.QueryInterface(Ci.nsICacheTesting);
+ testingInterface.clearDictionaryCacheMemory();
+
+ await syncCache();
+
+ let receivedAvailableDictionary = null;
+
+ // Register dictionary endpoint
+ await server.registerPathHandler(
+ "/dict/hash-test",
+ function (request, response) {
+ response.writeHead(200, {
+ "Content-Type": "application/octet-stream",
+ "Use-As-Dictionary":
+ 'match="/hash-match/*", id="hash-test-dict", type=raw',
+ "Cache-Control": "max-age=3600",
+ });
+ response.end("DICTIONARY_FOR_HASH_TEST", "binary");
+ }
+ );
+
+ // Register matching resource endpoint
+ await server.registerPathHandler(
+ "/hash-match/test",
+ function (request, response) {
+ global.lastHashTestAvailDict =
+ request.headers["available-dictionary"] || null;
+ response.writeHead(200, {
+ "Content-Type": "text/plain",
+ "Cache-Control": "no-cache",
+ });
+ response.end("HASH_MATCHING_CONTENT", "binary");
+ }
+ );
+
+ dump("**** Step 1: Load dictionary resource with Use-As-Dictionary\n");
+
+ let dictUrl = `https://localhost:${server.port()}/dict/hash-test`;
+ let chan = makeChan(dictUrl);
+ let [, data] = await channelOpenPromise(chan);
+
+ Assert.equal(
+ data,
+ "DICTIONARY_FOR_HASH_TEST",
+ "Dictionary content should match"
+ );
+ await verifyDictionaryStored(dictUrl, true);
+ await syncCache();
+
+ dump("**** Step 2: Verify Available-Dictionary is sent\n");
+
+ let matchingUrl = `https://localhost:${server.port()}/hash-match/test`;
+ chan = makeChan(matchingUrl);
+ await channelOpenPromise(chan);
+
+ receivedAvailableDictionary = await server.execute(
+ "global.lastHashTestAvailDict"
+ );
+ Assert.notStrictEqual(
+ receivedAvailableDictionary,
+ null,
+ "Available-Dictionary should be sent initially"
+ );
+
+ dump("**** Step 3: Corrupt the dictionary hash\n");
+
+ testingInterface.corruptDictionaryHash(dictUrl);
+
+ dump("**** Step 4: Clear dictionary data to force reload from disk\n");
+
+ // Clear dictionary data while keeping the corrupted hash.
+ // When next prefetch happens, data will be reloaded and compared
+ // against the corrupted hash, causing a mismatch.
+ testingInterface.clearDictionaryDataForTesting(dictUrl);
+
+ dump(
+ "**** Step 5: Request matching resource - should fail due to hash mismatch\n"
+ );
+
+ await server.execute("global.lastHashTestAvailDict = null");
+
+ // The request for the matching resource will try to prefetch the dictionary,
+ // which will fail due to hash mismatch. The channel should be cancelled.
+ chan = makeChan(matchingUrl);
+ try {
+ await channelOpenPromise(chan);
+ } catch (e) {
+ dump(`**** Request failed with: ${e}\n`);
+ }
+
+ // Note: The request may or may not fail depending on timing. The important
+ // thing is that the dictionary entry should be removed.
+
+ dump("**** Step 6: Verify dictionary entry was removed\n");
+
+ // Wait a bit for the removal to complete
+ await syncCache();
+
+ await server.execute("global.lastHashTestAvailDict = null");
+ chan = makeChan(matchingUrl);
+ await channelOpenPromise(chan);
+
+ receivedAvailableDictionary = await server.execute(
+ "global.lastHashTestAvailDict"
+ );
+ Assert.equal(
+ receivedAvailableDictionary,
+ null,
+ "Available-Dictionary should NOT be sent after dictionary was removed due to hash mismatch"
+ );
+
+ dump("**** Test passed: Hash mismatch properly handled\n");
+});
+
// Cleanup
add_task(async function cleanup() {
let lci = Services.loadContextInfo.custom(false, {
