HashMap iterator use-after-free in WebSWClientConnection::notifyRecordResponseBodyChunk
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
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)
Significance
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.
Audit directions
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?"