[6] PathCG should use thread-safe scratchContext

MediumWebCore platform graphicsRace

Two locks, one CoreGraphics context, and they never excluded each other.

2917a66

Medium — the prior fix's two locks never excluded each other, so a shared CoreGraphics context stays concurrently mutable from web content. Confirmed impact is a reliably reachable renderer crash; a stronger primitive depends on the raced graphics-state field being the heap-backed dash array, which the closed-source CG internals do not let us confirm.

Answering geometric questions about a stroked path — does this point lie on the stroke, what are the stroke's bounds — requires a CoreGraphics context to hold the stroke parameters, so WebKit keeps a throwaway "scratch" context that is never drawn to screen. Historically that context was a process-wide singleton, safe because only the main thread computed stroke geometry. OffscreenCanvas transferred into a Worker changed that: isPointInStroke() now runs on worker threads while the main thread computes SVG stroke bounding boxes for paint, and the invariant that only one thread mutates the scratch context at a time is what has to hold.

The angle: a page can drive two threads into concurrent unsynchronized mutation of one shared CoreGraphics context, corrupting graphics-state structures the renderer then reads.

The fix of bug 313935 was incomplete. That fix added two static Locks to PathCG::strokeContains() and PathCG::strokeBoundingRect(), making these two functions thread-safe independently. But it is possible to call strokeContains() and strokeBoundingRect() from two different threads at the same time. The fix is to make scratchContext() return a thread-specific graphics context, so two threads can't access the same context at the same time.

Source/WebCore/platform/graphics/cg/PathCG.cpp

LayoutTests/fast/canvas/offscreen-isPointInStroke-svg-stroke-bounds-race-crash.html

Patch Details

WebCore::scratchContext() is rewritten so the scratch CGContextRef is per-thread rather than process-global. The old implementation held a single static NeverDestroyed<RetainPtr<CGContextRef>> initialized once via createScratchContext(); the new one holds a static NeverDestroyed<ThreadSpecific<RetainPtr<CGContextRef>>> and lazily creates a context on first use by each thread (auto& result = *context.get(); if (!result) result = createScratchContext();).

With the shared context gone, the two function-local static Lock scratchContextLock / Locker locker { scratchContextLock } pairs previously added to strokeContains() and strokeBoundingRect() are deleted; both functions now call scratchContext() and proceed directly to CGContextSaveGState(context). The commit also adds a regression test that spawns 8 workers driving OffscreenCanvas.isPointInStroke() in tight loops with varying dash patterns while the main thread repeatedly mutates 20 SVG <path> elements styled with clip-path: inset(0) stroke-box to force strokeBoundingBox() recomputation on every paint.

Serializing access to one shared mutable resource with separate per-call-site locks, leaving disjoint critical sections that never exclude one another.

Background

CGContextRef and the graphics state stack. A CoreGraphics context carries a stack of graphics states — line width, line dash pattern, caps, joins, CTM — manipulated by CGContextSaveGState / CGContextRestoreGState, plus a mutable current path. CGContextSetLineDash copies the caller's dash array into the current graphics state, so the gstate holds a heap-backed buffer whose size depends on the caller-supplied dash count.

Scratch context pattern. PathCG needs a CGContext to answer purely geometric questions — CGContextReplacePathWithStrokedPath converts the current path into its stroked outline according to the current stroke gstate — so it keeps a throwaway context that is never presented to screen. createScratchContext() builds it; scratchContext() hands it out.

NeverDestroyed<T> and ThreadSpecific<T>. NeverDestroyed<T> is a WTF wrapper for a function-local static whose destructor is never run, giving a leak-on-purpose process-lifetime singleton. ThreadSpecific<T> provides one instance of T per thread, backed by thread-local storage; *threadSpecific returns the calling thread's own instance, default-constructed on first access and destroyed at thread exit.

Function-local static Lock. A Lock declared inside a function body is a single object shared across all invocations of that function, but a separate Lock declared inside a different function is an entirely distinct object.

OffscreenCanvas in workers. canvas.transferControlToOffscreen() produces an OffscreenCanvas that can be postMessage'd to a Worker with transfer, after which the worker thread owns the 2D context and its API calls — including isPointInStroke() — execute on the worker thread rather than the main thread.

stroke-box reference box. A clip-path whose reference box is stroke-box is resolved against the element's stroke bounding box, so the renderer must compute strokeBoundingBox() for that element during paint; mutating the d and stroke-dasharray attributes invalidates any cached value and forces recomputation on the next frame.

Analysis

This is a data race on shared mutable state, with memory-unsafety consequences inside CoreGraphics's own allocations.

  Worker thread                          Main/rendering thread
  ─────────────                          ─────────────────────
  strokeContains()                       strokeBoundingRect()
    Locker { lockA }  ◄── distinct ──►     Locker { lockB }
    ctx = scratchContext() ─┐           ┌─ ctx = scratchContext()
                            ├── SAME ───┤
    CGContextSaveGState(ctx)│  CGContextRef │  CGContextSaveGState(ctx)
    CGContextSetLineDash(...)             CGContextSetLineDash(...)
    CGContextReplacePathWithStrokedPath   CGContextReplacePathWithStrokedPath
    CGContextRestoreGState(ctx)           CGContextRestoreGState(ctx)
           └──── interleaved gstate push/pop and dash-buffer writes ────┘

Both functions mutate the context heavily — CGContextSaveGState, the stroke-style applier (line width, dash via CGContextSetLineDash, joins, caps), path construction, and CGContextReplacePathWithStrokedPath. The earlier fix for bug 313935 attempted to serialize this by declaring a static Lock scratchContextLock inside each of the two functions. Because each function-local static is a distinct Lock object, the two critical sections were mutually disjoint: strokeContains() held lock A while strokeBoundingRect() held lock B, and neither excluded the other. The invariant that was supposed to hold — at most one thread mutates the shared scratch context at a time — was enforced only pairwise within each function, not across the resource.

A CGContext's graphics-state stack and current path are non-atomic mutable structures. Interleaved push/pop pairs can pop a gstate pushed by the other thread, and gstate fields backed by heap allocations — most visibly the line-dash array copied in by CGContextSetLineDash, which the regression test deliberately varies in length and content on both sides — can be overwritten or released by one thread while the other still references them. The fix removes the shared resource entirely rather than widening the lock: each thread gets its own context from ThreadSpecific, so the disjoint-critical-section problem is dissolved and the locks become unnecessary.

The regression test is itself the trigger recipe and is fully web-reachable with no special privileges:

1. createOffscreenCanvasWorkers() creates 8 canvases, calls transferControlToOffscreen() on each, and postMessages the handle to a fresh Worker. Each worker enters an unbounded while (true) loop that builds a 15-segment bezier path, calls ctx.setLineDash(dashes) with a dash count cycling 2..7 and per-iteration-varying dash lengths, then calls ctx.isPointInStroke(64, 64).
2. Each isPointInStroke lands in PathCG::strokeContains(), which under the old code took its own function-local lock, fetched the shared CGContextRef, and ran CGContextSaveGState followed by the stroke-style applier — pushing the worker's dash array into the shared gstate.
3. On the main thread, createSVGPaths() builds 20 <path> elements each styled clip-path: inset(0) stroke-box, forcing strokeBoundingBox() during every paint.
4. animationLoop() rewrites every path's d and stroke-dasharray on each requestAnimationFrame, invalidating the cached stroke bounds so the next paint calls PathCG::strokeBoundingRect() — which took the other, unrelated lock and then mutated the same shared context.

Because the two locks are distinct objects, steps 2 and 4 run concurrently with zero mutual exclusion over the context. The test's comment — "Vary dash pattern to ensure CGContextSetLineDash allocates different arrays" — indicates the author was steering the race toward the heap-backed dash-array field of the graphics state rather than toward benign scalar fields.

The directly evidenced primitive is a controlled crash of the rendering process, reachable with high reliability. Beyond that: the two racing call sites both execute CGContextSaveGState(context) on a shared context, so interleaved push/pop sequences could desynchronize the graphics-state stack, and CGContextSetLineDash with attacker-chosen dash counts on both sides could cause one thread's gstate dash buffer to be replaced or released while the other still references it. If that torn-gstate condition holds, it would amount to a use-after-free or double-free of a CoreGraphics-internal allocation whose size the attacker influences through the dash-array length; realizing it would require winning a narrow timing window repeatedly and grooming the CG allocator. CoreGraphics is closed-source and its graphics-state and dash-buffer allocation behaviour is not observable from this change, so the step from torn graphics state to a usable heap primitive is a projection — anchored on the CGContextSetLineDash / CGContextSaveGState calls the diff and the regression test exercise, but not demonstrated by either.

Both functions are geometric queries executed in WebCore alongside layout and canvas state, i.e. in the WebContent process, and the racing threads are a WebContent worker thread and the WebContent main/rendering thread. No sandbox boundary is crossed; an attacker would still need a separate escape to leave WebContent.

This vulnerability weakens memory safety within the rendering process by breaking the thread-confinement assumption that a shared CoreGraphics scratch context is only ever mutated by one thread at a time. The security-model assumption at stake is that a process-global mutable object touched from both the main rendering thread and worker threads is fully serialized; before the fix that serialization was only partial. An attacker running script that drives isPointInStroke() from workers while forcing stroke-bounding-box recomputation on the main thread could reach corrupted CoreGraphics-internal state; at minimum this permits a reliably reachable crash of the rendering process, and if the corrupted state includes a heap-backed gstate field such as the line-dash array, a stronger memory-corruption primitive could follow.

Insight

This is a textbook incomplete-fix follow-up, and the failure mode is instructive: the original patch placed the lock at the call site rather than next to the resource. Two function-local static Lock objects look, at a glance, like "the scratch context is locked," but they are two independent mutexes over one object — the classic shape where a reviewer's eye reads the Locker line and stops. The eventual fix is the more durable one: instead of widening the lock to cover the resource, it removes sharing altogether via ThreadSpecific, converting a synchronization problem into a confinement problem. That tradeoff is worth remembering whenever a "shared scratch object" pattern meets multi-threaded reachability. The broader trend it reflects: features like OffscreenCanvas-in-workers retroactively promote long-standing main-thread-only globals into multi-threaded reachability, and each such global is a latent bug until audited.

Audit directions

• A lock declared at the use site instead of beside the data it protects, producing multiple disjoint critical sections over one shared resource. The invariant is a mutable resource must be guarded by exactly one lock, and every mutation site must take that same lock — hard to hold because a function-local static Lock reads locally as correct while being globally meaningless. Narrow: grep Source/WebCore/platform/graphics/ for static Lock declared inside a function body and check whether the object it guards is a function-local static/NeverDestroyed singleton reachable from any other function — start with the other path backends (PathCairo.cpp, PathSkia.cpp) and with FontCascade/glyph-cache helpers that use the same scratch-object idiom. Wider: the same class appears through different mechanisms — a std::once_flag-guarded initializer paired with unguarded later mutation, an atomic flag used to guard a non-atomic composite structure, or two different Locker scopes naming different lock members of the same owner; the tell in code-search results is any function whose lock object is declared in a narrower scope than the data it names. Widest: lock granularity must be keyed to the resource, not to the caller — carry it into any codebase with process-global mutable singletons, e.g. Chromium's base::NoDestructor<T> + separately declared base::Lock, Rust's static Mutex<()> guarding a separate static mut, or Java synchronized instance methods that mutate shared static fields. Match tell on every rung: count the distinct lock objects that can be held while the same resource is mutated; if that count is greater than one, it is a hit. In code review, a static Lock declared inside a function body deserves a comment naming the resource it protects and every other function that takes it.

• A global that was safe under a single-threaded execution model and became unsafe when a new API surface made it reachable from additional threads. The invariant is every mutable file-scope or function-local static must be either immutable, confined, or fully serialized once any worker-thread entry point can reach it. Narrow: audit Source/WebCore/platform/graphics/ and Source/WebCore/html/canvas/ for function-local static NeverDestroyed<...> holding mutable platform objects (contexts, buffers, caches) reachable from the canvas 2D geometry entry points — isPointInStroke, isPointInPath, measureText, and the stroke/fill bounding-box paths — and confirm each is either ThreadSpecific or covered by a single resource-scoped lock. Wider: enumerate the entry points that recently gained worker-thread reachability — OffscreenCanvas (2D and WebGL), createImageBitmap, CSS Painting API worklets, WebCodecs — and walk each call graph looking for the first mutable process-global it touches; the shape to notice is a helper named scratch*, shared*, or *Cache with no locking anywhere in the file. Widest: reachability expansion invalidates prior thread-safety proofs — worth carrying into Gecko's OffscreenCanvas work, any library adding a thread pool over a legacy singleton, or a service that moves a request handler from single-worker to multi-worker deployment. Match tell: a mutable static whose only documented safety argument is "main thread only" and whose callers now include a non-main-thread frame.

• Incomplete remediation, where a fix is applied at the reported crash site rather than to the violated invariant, leaving sibling call sites unprotected. The invariant is a fix must cover every path that can reach the broken state, not just the one in the crash report. Narrow: for PathCG specifically, re-walk every remaining function in PathCG.cpp and GraphicsContextCG.cpp that obtains a process-wide CoreGraphics object and confirm none reintroduces sharing; then check whether the sibling backends' equivalents of strokeContains/strokeBoundingRect received the same treatment. Wider: review WebKit security fixes whose entire diff is "add a Locker to one function" and ask, for each, how many other functions mutate the same guarded object — the code-search shape is a commit touching exactly one function body plus a lock declaration, with no change to the resource's declaration; a fix that instead changes the resource's ownership (to ThreadSpecific, per-object, or immutable) is the stronger form. Widest: did this fix change the reachable state space or only one route into it? — applicable to any codebase's post-mortem culture, including CVE follow-ups where a bounds check was added to one caller of a shared helper rather than to the helper. Match tell: the patched function is one of several that write the same object, and the patch does not touch that object's declaration.

• Verify the lifetime semantics of the newly introduced ThreadSpecific<RetainPtr<CGContextRef>> — thread-local storage holding a reference-counted or framework-owned object, where destruction runs at thread exit on a teardown path with unusual state (no autorelease pool, partially torn-down runtime). Narrow: confirm the RetainPtr<CGContextRef> release at worker-thread exit is safe and that NeverDestroyed<ThreadSpecific<...>> ordering does not leave a dangling TLS key on process shutdown — the tell is a CFRelease/-release reached from a TLS destructor. Wider: grep for other ThreadSpecific<> instantiations in WebCore whose element type owns a CoreFoundation, Objective-C, or GPU handle rather than a plain POD, and check each for the same teardown assumption. Widest: thread-local destructors run in a degraded context and must not depend on ambient runtime state, which holds for POSIX pthread_key_create destructors, Rust's thread_local! with Drop impls, and Java ThreadLocal in pooled-thread environments. Match tell: a TLS-held type with a non-trivial destructor that calls into a framework rather than just freeing memory. Verification here is nontrivial — it likely needs a worker-churn stress test under a leak/ASan build rather than static inspection.

• Memory growth as a side effect of converting shared state to per-thread state. Now that each thread creating stroke geometry allocates its own CGContext via createScratchContext(), examine whether a page can force unbounded thread creation (many Workers each touching isPointInStroke) into a proportional number of live CoreGraphics contexts. Narrow: check the allocation size of createScratchContext() in PathCG.cpp and multiply by a realistic worker cap. Wider: the shape to look for elsewhere is any recent shared-to-ThreadSpecific conversion where the per-thread object is large and thread count is attacker-influenced. Widest: confinement trades contention for per-actor memory, and per-actor memory is an availability surface whenever the actor count is attacker-controlled — applies to per-connection buffers in servers, per-goroutine arenas, and thread-local allocator caches generally. Match tell: a ThreadSpecific/thread_local whose element allocates non-trivially and whose owning thread is spawnable from untrusted input.