[5] Tracker lookup tables raced between the WebPrivacy and resolver threads

MediumWebKit Advanced Privacy ProtectionsRace

f837067

Medium. Two process-global tables were searched on the networking stack's resolver thread while another thread cleared and rebuilt them, with one path additionally handing out an interior const char* and another releasing non-atomic refcounts outside the lock. Turning that into a controlled free requires a refresh landing inside a lookup window the attacker does not get to schedule.

Data races on shared containers are the usual route from "two threads, one table" to a use-after-free: one thread reallocates the backing store while another is indexing into it. WebKit's Advanced Privacy Protections maintain process-global tables of known tracker IP ranges and registrable domains inside the Network process, populated from Apple's WebPrivacy framework. Those tables are also registered as a lookup callback on the nw_context_t behind an NSURLSession, so the system networking stack queries them from its own resolver thread while WebKit refreshes them from the WebPrivacy thread.

The angle: an attacker able to drive network traffic while a tracker-table refresh lands can get the resolver thread to binary-search a freed or shrinking vector and to follow a byte pointer into a buffer whose last owner has gone away.

Patch Details

Three distinct defects in WebPrivacyHelpers.mm are closed by one patch, and the fix's shape is as instructive as the bug.

The address tables — TrackerAddressLookupInfo's version4List() and version6List(), both NeverDestroyed<Vector<TrackerAddressLookupInfo>> — had no lock at all: populateIfNeeded()'s completion handler called version4List().clear(); version6List().clear(); and rebuilt them while find() binary-searched the same vectors. The patch serializes refresh and lookup under a new trackerLookupLock() and encodes the requirement in the type system with WTF_REQUIRES_LOCK annotations on matchingInfo(), version4List() and version6List().

The domain table — a NeverDestroyed<MemoryCompactRobinHoodHashMap<String, TrackerDomainLookupInfo>> — did have its own domainListLock, but find() was declared static const TrackerDomainLookupInfo find(String host), returning the entry by value so the copy and its refcounted string buffers escaped the critical section with the caller. The patch makes matchingInfo() private and restructures find() to take the lock internally and hand the entry to a NOESCAPE callback rather than return it.

Both paths in the setTrackerLookupCallback block published info.owner().legacyCStringPointer() and info.host().legacyCStringPointer() into const char** out-parameters — raw pointers into storage the tables own. What the callback now publishes is a pointer into a thread_local UTF8CString copy, produced via a new isolatedCopy() on WTF's CString / CStringWithEncoding.

  WebPrivacy thread                  Resolver thread (nw_context callback)
  ─────────────────                  ────────────────────────────────────
  populateIfNeeded() completes
  version4List().clear()       ──┐   find(address)
  append(...)   (realloc)        │     list.isEmpty()          (read pre-clear)
  append(...)                    │     list[mid], list[upper]  ← freed/shrunken
                                 └──►  info.containsAddress()
                                         reads m_network, m_netMaskLength
                                       *owner = info->owner()
                                                .legacyCStringPointer()  ← escapes

Process-global mutable tables read on one thread and rebuilt on another, with an interior pointer and a non-atomic refcount both escaping the critical section.

Background

Where this lives. Source/WebKit/Platform/cocoa/WebPrivacyHelpers.mm bridges Apple's WebPrivacy framework tracker data into WebKit's networking path, in the Network process. It maintains process-global tables of known tracker IP ranges and registrable domains, exposes them to WebKit policy code through isKnownTrackerAddressOrDomain and isRequestBlockable, and registers a lookup callback on the nw_context_t backing an NSURLSession so the system networking stack can attribute and optionally block connections.

Two threads, by design. Per the commit message, the setTrackerLookupCallback block runs on the networking stack's resolver thread, while the tables are populated and refreshed on the WebPrivacy thread. Neither thread is WebKit's main thread, and neither schedule is controlled by the other; the tables are the only thing they share.

Vector reallocation. WTF's Vector keeps elements in one contiguous buffer. clear() destroys the elements; a rebuilding sequence of append() calls grows the buffer, and growth means allocating a new block, moving elements into it, and freeing the old one. An index computed against one buffer is meaningless against another, and a read through the freed one is a read of freed heap.

CString, CStringBuffer and refcount atomicity. CString holds a RefPtr<CStringBuffer> m_buffer, and class CStringBuffer final : public RefCounted<CStringBuffer> — plain RefCounted, not ThreadSafeRefCounted. The increment and decrement are ordinary non-atomic memory operations, which is correct and fast as long as every reference to a given buffer lives on one thread. legacyCStringPointer() returns a raw const char* into that buffer's bytes, owned by whoever holds the CString.

isolatedCopy() and NOESCAPE. isolatedCopy() is WTF's idiom for producing a value whose backing allocation is not shared with the original, so it can safely cross a thread boundary — the thread-safety equivalent of a deep copy. NOESCAPE on a callback parameter states that the callee does not retain the callable past the call, which is what lets a lock be held for the callback's whole duration without risking a later, unlocked invocation.

Analysis

The root cause is three overlapping failures of the same invariant — shared mutable state must be accessed under a lock that covers every use of what the access produced — and they escalate differently, which is why the fix addresses all three rather than adding one lock.

Strand one: unsynchronized container access. The pre-fix find() read list.isEmpty() and then indexed list[mid], list[upper], list[lower]. A concurrent clear() destroys those entries; a rebuilding append sequence reallocates the backing store. Either turns the indices into reads of freed or shrunken memory, after which containsAddress() reads m_network and m_netMaskLength out of whatever now occupies the block. This is the out-of-bounds/use-after-free half of the bug, and the WTF_REQUIRES_LOCK annotations are what stop it from reappearing: the requirement is now checked rather than remembered.

Strand two: escaping interior pointer. *owner = info->owner().legacyCStringPointer() handed the networking stack a const char* pointing into the matched entry's CStringBuffer. Nothing kept that entry alive past the call, so the next refresh destroyed it and dropped the last reference to the buffer while the networking stack still held the pointer. The fix does not merely document the hazard away; it removes the ability to make the mistake by making matchingInfo() private, keeping the entry inside a lock-held NOESCAPE callback, and publishing a thread_local copy instead of an interior pointer.

Strand three: non-atomic refcount across threads. The domain path's by-value return did copy the entry under domainListLock — but the copy was destroyed in the caller after the Locker went out of scope, so its CStringBuffer and StringImpl releases ran unsynchronized against the WebPrivacy thread's list().set(...) releases of the same objects. Because these are plain RefCounted, the updates are non-atomic, and a lost update either leaks the buffer or frees it while the other thread still holds a reference. This is precisely why the fix reaches for isolatedCopy() — a fresh buffer per thread — rather than simply copying the CString, which would have shared the buffer and preserved the race.

One refinement on the commit message's framing of strand two in the domain path: because CStringBuffer is refcounted and the returned copy shared the buffer with the live map entry, the bytes did not necessarily become free the instant the temporary was destroyed at the end of the if statement. What disappeared at that moment was any ownership guarantee keeping them alive. The memory was reclaimed at the next refresh — or earlier, if strand three dropped an increment.

Exploitability is gated by scheduling the attacker does not own. The refresh cadence is driven by the WebPrivacy framework rather than by page activity, so hitting the window means driving lookups continuously and waiting for a refresh to land inside one, and the reclaim contents are whatever the Network process's allocator hands back. The primitive the bug establishes on its own is a read of freed or reallocated table memory and a non-atomic refcount update racing another thread's; escalation past that would require controlling what fills the reclaimed block, which this change does not establish.

This vulnerability weakens the Network process's internal consistency in a code path that runs on a thread WebKit does not own, on data structures that participate in privacy policy decisions.

Audit directions

Lookups that return by value out of a locked region. A find() that takes a lock, copies an entry, and returns the copy has moved the entry's destructor — and every refcount release it performs — outside the critical section. The pattern is invisible at the call site and looks like good hygiene at the definition. Audit the other lookup helpers in WebPrivacyHelpers.mm and, more broadly, any static T find(...) over a process-global container in a multithreaded process. Code-review tell: a by-value return type on a function whose body constructs a Locker.
Interior pointers published across a callback boundary. legacyCStringPointer(), characters(), data() and span().data() all hand out addresses owned by something else; publishing one into a const char** out-parameter for a framework to hold is a lifetime contract nobody wrote down. Look for raw byte pointers crossing into system-framework callbacks, especially where the source object lives in a table another thread refreshes, and check whether the fix shape here — thread_local copy plus isolatedCopy() — is available at those sites.
Plain RefCounted objects reachable from two threads. RefCounted versus ThreadSafeRefCounted is a load-bearing distinction that no compiler diagnostic enforces. Enumerate the types stored in process-global tables consulted from system-framework callback threads and check each one's refcount base class; CString/CStringBuffer and String/StringImpl are the two that show up most often in exactly this position.
Lock requirements that exist only in comments. The WTF_REQUIRES_LOCK annotations added here are the durable half of the fix. Where a global accessor is documented as "call under the lock" without the annotation, treat every caller as unaudited, and prefer adding the annotation to reading the callers — the compiler will find the ones you would have missed.