← All issues

[14] SWServer dereferences unchecked HashMap end iterator

Severity: Low | Component: WebCore Service Worker Server | 3bce213

Rated Low because the diff fixes a HashMap end-iterator dereference reachable only when paired client maps desynchronise across a race the commit message states has no known reproducer; impact is bounded to a crash in the service-worker bookkeeping path, with no evidence of attacker-controlled read/write.

A crash occurs in topLevelServiceWorkerClientFromPageIdentifier() when the Service Worker maps m_clientIdentifiersPerOrigin and m_clientsById become out-of-sync. The function iterates over client identifiers from m_clientIdentifiersPerOrigin and calls m_clientsById.find() for each one, but never checks whether the result equals m_clientsById.end() before dereferencing.

Source/WebCore/workers/service/server/SWServer.cpp

auto clientIterator = m_clientsById.find(clientIdentifier);
-ASSERT(clientIterator != m_clientsById.end());
+if (clientIterator == m_clientsById.end()) [[unlikely]] {
+ ASSERT_NOT_REACHED();
+ return std::nullopt;
+}
return clientIterator->value;
...
for (auto clientIdentifier : iterator->value.identifiers) {
auto clientIterator = m_clientsById.find(clientIdentifier);
+ if (clientIterator == m_clientsById.end()) {
+ ASSERT_NOT_REACHED();
+ continue;
+ }
if (clientIterator->value->frameType == ServiceWorkerClientFrameType::TopLevel && clientIterator->value->pageIdentifier == pageIdentifier)
return clientIterator->value;
}

In serviceWorkerClientWithOriginByID(), the previous ASSERT(clientIterator != m_clientsById.end()) is replaced with a real end-iterator check that returns std::nullopt in release builds. In topLevelServiceWorkerClientFromPageIdentifier(), the loop that iterates over iterator->value.identifiers and calls m_clientsById.find(clientIdentifier) previously dereferenced unconditionally; an end-iterator check with ASSERT_NOT_REACHED() and continue is added so missing entries are skipped.

Dereferencing a HashMap iterator without an end-check when paired-map invariants can desynchronize across asynchronous lifecycle paths.

SWServer is the cross-process owner of service-worker state for a session. m_clientIdentifiersPerOrigin is a HashMap<ClientOrigin, ...> whose value type carries an identifiers set listing every ScriptExecutionContextIdentifier that has registered as a client of that origin. m_clientsById is a separate HashMap<ScriptExecutionContextIdentifier, RefPtr<ServiceWorkerClientData>> keyed by the same identifier. The intended invariant is that every identifier appearing in any per-origin identifiers set has a corresponding entry in m_clientsById. HashMap::find() returns an iterator equal to end() when the key is not present, and dereferencing that end iterator is undefined behaviour. ASSERT(...) in WebCore is compiled out in release builds, so an ASSERT-only guard provides no runtime protection.

topLevelServiceWorkerClientFromPageIdentifier() obtained a client identifier from the per-origin set in m_clientIdentifiersPerOrigin and then performed m_clientsById.find(clientIdentifier) without checking whether the returned iterator equals m_clientsById.end(). The function then dereferenced clientIterator->value. The two maps are populated and torn down through separate code paths — the commit message describes this as a race that can leave them out-of-sync.

🔒

The lifetime and synchronization implications between two parallel service-worker bookkeeping maps are explored, along with the realistic ceiling on what this crash can be escalated to.

Subscribe to read more

🔒

Multiple reusable audit patterns identified across the service-worker subsystem, with concrete grep targets for finding sibling instances of the same defensive gap.

Subscribe to read more