ad57b514bb  Potential use after free of m_stream in ReadableStreamDefaultReader::visitAdditionalChildren()

The bug at a glance

ReadableStreamDefaultReader is created by any page that calls stream.getReader(), and its wrapper is visited by the concurrent GC thread which dereferenced m_stream while the main thread could null it via releaseLock/genericRelease/destructor. That is a cross-thread UAF of a live JS-reachable ReadableStream, reachable from untrusted content, hence high.

visitAdditionalChildrenInGCThread and isReachableFromOpaqueRoots run on the GC thread and dereferenced m_stream, while releaseLock() used std::exchange(m_stream, {}) and genericRelease() set m_stream = nullptr on the main thread with no synchronization. The fix adds m_streamLock and routes every m_stream access - reads, writes, and the GC visitor - through it.

Root cause

A ReadableStreamDefaultReader holds a RefPtr<ReadableStream> m_stream. On the concurrent GC marking thread, ReadableStreamDefaultReader::visitAdditionalChildrenInGCThread(visitor) checks if (m_stream) m_stream->visitAdditionalChildrenInGCThread(visitor), and isReachableFromOpaqueRoots() reads m_stream && m_stream->isReachableFromOpaqueRoots(). Both dereference m_stream without a lock.

On the main thread, releaseLock() did RefPtr stream = std::exchange(m_stream, { }) and genericRelease() did m_stream = nullptr; the destructor read RefPtr stream = m_stream. When script calls reader.releaseLock() (or drops the reader so ~ReadableStreamDefaultReader runs) concurrently with a GC marking pass, the exchange/assignment can free the ReadableStream just as the GC thread loads the old pointer and calls into it.

Because the ReadableStream’s own visitAdditionalChildrenInGCThread / isReachableFromOpaqueRoots are then invoked on a freed object, the GC thread reads a dangling vtable and member state - a use-after-free during marking.

The patch adds mutable Lock m_streamLock with m_stream WTF_GUARDED_BY_LOCK(m_streamLock). Every reader (read, setup, genericRelease, genericCancel, cancel, destructor, the new out-of-line stream() accessor) now snapshots m_stream into a local RefPtr under the lock; every writer (releaseLock’s std::exchange, genericRelease’s m_stream = nullptr) holds the lock; and both GC-thread functions (isReachableFromOpaqueRoots, visitAdditionalChildrenInGCThread) take the lock before touching m_stream. ReadableStream::Iterator::isFinished and the two GC-thread functions gain SUPPRESS_NODELETE/SUPPRESS_NODELETE-style annotations to satisfy the checker now that stream() is a locked out-of-line call.

Key code

Lock-guarded GC-thread accessors in ReadableStreamDefaultReader.cpp

ReadableStream* ReadableStreamDefaultReader::stream()
{
    Locker locker { m_streamLock };
    return m_stream.get();
}

SUPPRESS_NODELETE bool ReadableStreamDefaultReader::isReachableFromOpaqueRoots() const
{
    Locker locker { m_streamLock };
    return getNumReadRequests() && m_stream && m_stream->isReachableFromOpaqueRoots();
}

template<typename Visitor>
void ReadableStreamDefaultReader::visitAdditionalChildrenInGCThread(Visitor& visitor)
{
    Locker locker { m_streamLock };
    if (m_stream)
        SUPPRESS_UNCOUNTED_ARG m_stream->visitAdditionalChildrenInGCThread(visitor);
}

Patch walkthrough

• Source/WebCore/Modules/streams/ReadableStream.cpp — ReadableStream::Iterator::isFinished() is marked SUPPRESS_NODELETE because it now calls the out-of-line, lock-taking m_reader->stream() rather than the old trivial inline getter.
• Source/WebCore/Modules/streams/ReadableStreamDefaultReader.cpp — Every m_stream access is wrapped in Locker{m_streamLock}: destructor, read(), releaseLock() (now takes the lock around the internalReader->releaseLock()/std::exchange sequence), setup(), genericRelease() (locks the final m_stream = nullptr), cancel(), genericCancel(). Adds an out-of-line ReadableStream* stream() that returns m_stream.get() under the lock, and makes isReachableFromOpaqueRoots() and visitAdditionalChildrenInGCThread() take the lock before dereferencing m_stream.
• Source/WebCore/Modules/streams/ReadableStreamDefaultReader.h — Adds <wtf/Lock.h>, replaces the inline ReadableStream* stream() { return m_stream.get(); } with a declaration, and adds mutable Lock m_streamLock with RefPtr<ReadableStream> m_stream WTF_GUARDED_BY_LOCK(m_streamLock).

Background

ReadableStreamDefaultReader — The object returned by ReadableStream.getReader(). It holds a back-reference to its stream in m_stream, which is cleared when the lock is released (releaseLock) or the reader is torn down. It participates in GC opaque-root marking so the stream stays alive while the reader has pending read requests.

releaseLock / std::exchange — The spec ‘release lock’ step detaches the reader from its stream. The old code used std::exchange(m_stream, {}) to atomically swap and drop the reference on the main thread, but this is only atomic w.r.t. that single thread; a concurrent GC-thread read of m_stream is still a race.

isReachableFromOpaqueRoots — Called by the GC to decide whether the reader’s stream must be kept alive. Reading m_stream here off-thread while the main thread nulls it is the reachability-side manifestation of the same race as the visitor.

SUPPRESS_NODELETE / SUPPRESS_UNCOUNTED_ARG — WebKit static-analysis suppression macros. They are added where the new locked accessors would otherwise trip the smart-pointer/lifetime checkers, documenting that the raw dereference under the lock is intentional and safe.

Vulnerability window

1. Baseline — m_stream is a plain RefPtr accessed by both the main thread and the GC marking thread without synchronization.
2. Trigger — Script obtains a default reader, starts a read, then calls releaseLock() (or drops the reader) while allocation keeps a concurrent GC marking cycle running.
3. Race window — GC thread evaluates if (m_stream) / dereferences it in visitAdditionalChildrenInGCThread while the main thread’s std::exchange/assignment frees the ReadableStream.
4. UAF — GC thread calls into the freed ReadableStream’s visitAdditionalChildrenInGCThread/isReachableFromOpaqueRoots.
5. Fix — m_streamLock serializes all access; GC visitor and reachability check hold it while dereferencing m_stream.

Triggering

No regression test ships with this lifetime fix. Conceptual trigger: const r = stream.getReader(); kick off r.read(); then in a loop r.releaseLock() and re-acquire a reader while allocating aggressively to keep the concurrent GC marking thread active, so a marking pass dereferences m_stream exactly as releaseLock/genericRelease nulls it. Best observed under TSAN/ASAN; the race is probabilistic.

Exploitation

1. Reachability — Any page can call getReader()/releaseLock(); concurrent GC is always active, so the race is reachable without special privileges.
2. Window widening — Attacker interleaves releaseLock/reacquire with heavy allocation to keep the marker busy, and may hold pending read requests to steer isReachableFromOpaqueRoots evaluation.
3. Primitive — Freed ReadableStream dereferenced by the GC thread. As with the XHR sibling, this is realistically crash-only unless the freed slot is groomed and the race won; no weaponization is demonstrated.

Detection & hunting

For defenders and SOC / detection engineers:

• TSAN race on m_stream —
• ASAN UAF in stream marking —
• Crashes in reader opaque-root reachability —

Audit directions

• ReadableStreamBYOBReader —
• Streams objects with GC-visited back-references —
• std::exchange on GC-visited RefPtrs —
• Inline trivial getters exposed off-thread —