← All reports

HashMap iterator use-after-free in WebSWClientConnection::notifyRecordResponseBodyChunk

WebKit Service WorkersUAF

One half of the Service Worker protocol was fixed for re-entrancy; the other wasn't.

Component: WebKit Service Workers | 20af97e

Source/WebKit/WebProcess/Storage/WebSWClientConnection.cpp

void WebSWClientConnection::notifyRecordResponseBodyChunk(RetrieveRecordResponseBodyCallbackIdentifier identifier, IPC::SharedBufferReference&& data)
{
- auto iterator = m_retrieveRecordResponseBodyCallbacks.find(identifier);
- if (iterator == m_retrieveRecordResponseBodyCallbacks.end())
+ auto callback = m_retrieveRecordResponseBodyCallbacks.take(identifier);
+ if (!callback)
return;
auto buffer = data.unsafeBuffer();
bool isDone = !buffer;
- iterator->value(WTF::move(buffer));
- if (isDone)
- m_retrieveRecordResponseBodyCallbacks.remove(iterator);
+ callback(WTF::move(buffer));
+ if (!isDone)
+ m_retrieveRecordResponseBodyCallbacks.add(identifier, WTF::move(callback));
}

WebSWClientConnection brokers Service Worker IPC in the WebProcess, including delivering background fetch response body chunks to BackgroundFetchResponseBodyLoader, which feeds a ReadableStream backing a FetchResponse body. Callbacks for in-flight body retrievals are tracked in a HashMap keyed by a callback identifier — and HashMap iterators and references are invalidated by any insertion that triggers a rehash.

The old code found the entry, invoked iterator->value(...), and then used the same iterator again to remove() the entry. The new code take()s the callback out of the map first, invokes it, and re-inserts it only if streaming continues.

Before:                                          After:
find(id) -> iterator                            take(id) -> callback (removed from map)
  |                                                |
  iterator->value(data)  <-- may reenter           callback(data)  <-- may reenter
  |     and insert into same map, rehash!          |     safe: iterator no longer held
  |                                                |
  map.remove(iterator)  <-- iterator may be stale  if (!isDone) map.add(id, callback)

The callback here is arbitrary WebKit-internal code that can synchronously call back into the same connection object. If the invoked callback synchronously re-enters retrieveRecordResponseBody, the HashMap can rehash and invalidate the iterator used for the subsequent remove() — a use-after-free reachable through normal Service Worker background fetch streaming. The sibling function notifyRecordResponseBodyEnd had already been fixed to avoid exactly this via the take-then-invoke shape, so the fixed version restores consistency between two halves of the same protocol.

This is a public, patched instance of a broader re-entrancy bug class in WebKit's IPC callback map handling: invoking a stored Function<> while still holding a HashMap iterator or reference into the map that the callback can itself mutate. Narrow: grep WebKit for other find()iterator->value(...)remove(iterator) sequences (as opposed to take-then-invoke) around callback maps in IPC-facing classes — WebSWClientConnection, WebSWServerConnection, and similar background fetch, service worker, and network-process callback registries are the natural starting set. Any callback that can synchronously re-enter the same object and insert into the same map is a candidate for the identical UAF. In code review, the visual tell is an iterator declared from find() whose lifetime spans a call through a stored callable. Wider: the same shape covers any container iterator or reference held across a callback invocation, including Vector element references across a append-capable handler and HashSet iterators across notification dispatch. Widest: a container handle is only valid until the container can be mutated, and invoking arbitrary code is a mutation opportunity — the question to carry is "can this callback reach back into the collection I am iterating?"