[5] YARR RegularExpression out-of-bounds write with duplicate named capture groups

MediumJSC YARROOB

e536815

Rated Medium — the wrapper handed the byte-code interpreter a buffer shorter than the interpreter's own recorded requirement, so a pattern-chosen index writes past it. What keeps it out of the High band is that no in-tree caller with an attacker-influenced pattern string is established for this façade, and the container's over-allocation absorbs small overruns.

Regular-expression matching in JSC produces its results in an "offsets vector": a flat array of unsigneds where each capture group occupies two consecutive slots (start offset, end offset), with slot 0 and 1 reserved for the whole match. Two independent consumers drive the same engine — JSC::RegExp, which backs the JavaScript-visible RegExp object, and JSC::Yarr::RegularExpression, a simpler wrapper exported for callers outside JavaScriptCore. Both must hand the byte-code interpreter a vector at least as long as the byte code's own recorded requirement, because the interpreter indexes that vector directly by ids baked in at compile time.

The angle: a pattern with duplicate named capture groups drives the byte-code interpreter to write at indices past the end of the buffer the wrapper allocated — landing on match()'s stack frame or, for larger patterns, adjacent heap memory.

RegularExpression's offsets vector allocation size is incorrect: that formula was updated when named captures are added, but RegularExpression's computation was not updated correctly. This patch fixes it.

Source/JavaScriptCore/yarr/RegularExpression.cpp

Tools/TestWebKitAPI/Tests/JavaScriptCore/RegularExpression.cpp

Patch Details

One line of production code changes. In JSC::Yarr::RegularExpression::match(), the local offsetVectorSize — which sizes the Vector<unsigned, 32> nonReturnedOvector handed to Yarr::interpret() as the output vector — was computed with the wrapper's own formula (d->m_numSubpatterns + 1) * 2. The patch replaces that with the size the byte-code compiler itself recorded: d->m_regExpByteCode->m_offsetsSize.

Nothing else in match() changes. The nonReturnedOvector.grow(offsetVectorSize) call, the offsetNoMatch initialization loop (still bounded by m_numSubpatterns + 1), and the interpret() call are untouched. The remaining hunks are collateral: a new API test file with four TEST(JavaScriptCore_RegularExpression, DuplicateNamedCaptureGroup*) cases building RegularExpression objects over patterns with duplicate named capture groups under Flags::UnicodeSets, plus one-line registration in Tools/TestWebKitAPI/CMakeLists.txt. The tests are the trigger evidence: every one uses a pattern of the form (?<a>x)|(?<a>y), i.e. the same group name reused across alternatives.

A consumer re-deriving a buffer's required size with its own copy of a formula instead of reading the size the producer recorded.

Background

The offsets vector. YARR's match output is a flat array of unsigneds; for each capture group the interpreter stores a start offset and an end offset in two consecutive slots, with slot 0/1 reserved for the whole match. offsetNoMatch is the sentinel written into slots for groups that did not participate. YarrPattern::m_numSubpatterns counts the capturing groups, so the classic ovector length is (numSubpatterns + 1) * 2.

Duplicate named capture groups. A language feature permitting the same group name to appear more than once in a pattern as long as the occurrences are in mutually exclusive alternatives, e.g. (?<a>x)|(?<a>y). YARR tracks these with a per-name id and addresses their slots via BytecodePattern::offsetForDuplicateNamedGroupId(id); these slots sit after the conventional start/end pairs. BytecodePattern::m_offsetsSize is the byte-code object's record of how many unsigned slots the interpreter expects.

Two consumers of one engine. JSC::RegExp (runtime/RegExp.cpp) sizes its m_ovector from offsetVectorBaseForNamedCaptures() plus m_rareData->m_numDuplicateNamedCaptureGroups. JSC::Yarr::RegularExpression (yarr/RegularExpression.cpp) is an independent, simpler wrapper — a thin façade over YarrPattern → byteCompile() → interpret(), declared JS_EXPORT_PRIVATE so it is callable from outside JavaScriptCore — that allocates its own scratch vector per match() call.

Vector<T, N> inline capacity and growth policy. WTF's Vector template's second parameter is an inline capacity — up to N elements are stored in a buffer embedded in the Vector object itself, and only a request beyond N triggers a heap allocation. grow() routes through expandCapacity(), which reserves max(requested, max(16, capacity() + capacity() / 4 + 1)), so the reserved capacity after a heap growth is generally larger than the requested size. WTF's VectorTraits mark simple/POD element types as needsInitialization = false, so growing a Vector<unsigned> exposes elements whose contents are unspecified rather than zeroed. nonReturnedOvector is declared Vector<unsigned, 32> as a stack local of match().

interpret(). The YARR byte-code interpreter entry point; it takes the BytecodePattern, the subject string, a start offset, and a raw unsigned* output vector, and indexes that vector directly by the ids baked into the byte code.

Analysis

The root cause is a duplicated size formula that only one of its two copies got updated. When support for duplicate named capture groups was added, the offsets vector grew an additional trailing region addressed via offsetForDuplicateNamedGroupId(id); the producer records the true total in m_offsetsSize, and RegExp::finishCreation() consumes it correctly. The wrapper's hand-rolled (m_numSubpatterns + 1) * 2 was never touched.

  Byte code expects (m_offsetsSize):
  [ whole ][ sub1 ][ sub2 ] ... [ dupA ][ dupB ]
  |<------ (numSubpatterns+1)*2 ------>|<--- unallocated --->|
                                        ^
                          offsetForDuplicateNamedGroupId(id)
                          writes here, past what grow() asked for

The interpreter touches those slots directly by index: ParenthesesDisjunctionContext's constructor performs subpatternAndGroupIdBackup[...] = output[m_pattern->offsetForDuplicateNamedGroupId(duplicateNamedGroupId)] (a read) followed by output[pattern->offsetForDuplicateNamedGroupId(duplicateNamedGroupId)] = 0 (a write), and restoreOutput() writes the saved value back. With the stale formula, every such index lands at or past (m_numSubpatterns + 1) * 2.

What memory actually gets clobbered is governed by the vector's reserved capacity, not by the size passed to grow(), and the two diverge in both directions. While the stale size stays at or below 32, the storage is the embedded inline buffer and its capacity is exactly 32: a touched index of 32 or greater — roughly fifteen subpatterns plus several duplicate named groups — writes past the embedded buffer onto match()'s surrounding stack frame. Once the stale size exceeds 32, grow() routes through expandCapacity(), and growing out of a 32-element inline buffer reserves at least 41 elements; an overrun of a few slots past a stale requested size just above 32 consequently still lands inside the vector's own heap allocation, in its uninitialized slack, not on adjacent heap objects. Reaching adjacent heap memory requires the highest touched index to exceed the actual reserved capacity, which for larger patterns tracks roughly 1.25× the stale size. So the quiet window is wider than a naive reading suggests on the heap path, and narrower than it looks on the inline path.

There is a residual detail worth noting after the fix: the buffer is now m_offsetsSize long while the explicit offsetNoMatch seeding loop still stops at m_numSubpatterns + 1, leaving the trailing duplicate-group slots unseeded by the wrapper. WTF's VectorTraits mark simple/POD types as not needing initialization, so grow() does not zero them — the correctness of that gap rests on the interpreter initializing those slots before reading them, which it does for at least the ParenthesesDisjunctionContext path but which is not established for every path through interpret().

Discovery reads as pattern auditing or variant analysis rather than fuzzing: the fix is a single stale-formula line and the added tests are hand-written API tests all using Flags::UnicodeSets with duplicate named capture groups, which is the signature of someone deliberately enumerating consumers of the offsets-vector layout after the feature landed. Fuzzing is a less likely finder precisely because the smallest patterns — including the ones in the new tests — keep every touched index inside the inline buffer, and the heap growth policy over-allocates beyond the requested size, so no sanitizer report appears until a pattern pushes the touched index past reserved capacity.

This vulnerability weakens memory safety inside whichever process compiles a regular expression through the RegularExpression façade. The invariant broken before the fix is the contract between byteCompile() and interpret(): the output vector must hold at least m_offsetsSize unsigneds, and the wrapper silently supplied fewer whenever the pattern contained duplicate named capture groups. An attacker who could steer a pattern string into this API would obtain an out-of-bounds write at a pattern-chosen index whenever that index exceeds the vector's reserved capacity — landing on match()'s stack frame while the storage is still the embedded 32-element inline buffer, or on adjacent heap memory once the touched index runs past the over-allocated reserved heap capacity. Either would be a bounded corruption foothold, normally chained with grooming or with stack-layout knowledge rather than used directly. No in-tree caller feeding an attacker-influenced pattern string into this façade is identified here, which is what keeps the practical severity below that of the equivalent bug in the JS-visible RegExp path.

Takeaway: when triaging any "small overflow of a Vector<T, N>", compute the container's reserved capacity, not the requested length — below inline capacity the bound is exactly N and an overrun escapes onto the enclosing stack frame, while above it expandCapacity()'s over-allocation quietly absorbs modest overruns and hides them from ASan.

Audit directions

• A consumer re-deriving an allocation size with its own copy of a formula while the producer already records the authoritative size. The invariant is the party that decides a layout owns its size; consumers read it, they do not recompute it. Narrow: grep JavaScriptCore for the literal ovector arithmetic (+ 1) * 2, offsetVectorBaseForNamedCaptures, and every reader of BytecodePattern::m_offsetsSize — check RegExpInlines.h, RegExp::matchInline/matchConcurrently, RegExpMatchesArray, and the YarrJIT entry points, since each hands a caller-allocated unsigned* to a matcher that indexes by baked-in ids. Wider: wherever a size is published on an object but callers compute their own — ByteDisjunction::m_frameSize vs DisjunctionContext::allocationSize(), or any WebKit allocation whose length is derived from a count field rather than read from the structure being filled. Widest: any codebase where a compiler or serializer emits both data and a required-buffer-size field — LLVM stack frame size vs prologue emitters, protobuf/flatbuffer arena sizing, GPU descriptor-table sizing in Chromium's command buffer. In code review, the tell on every rung is a call site whose allocation argument is an arithmetic expression over counts, while the object it will be passed to exposes a ...Size member that the arithmetic is trying to reproduce.

• A language or format feature that widens an existing layout, where only some layout-size consumers get updated. The invariant is extending a struct's tail is a whole-program change, not a local one. Narrow: enumerate every consumer introduced or touched by duplicate-named-capture-group support — offsetForDuplicateNamedGroupId, m_numDuplicateNamedCaptureGroups, m_namedGroupToParenIndices — across yarr/YarrJIT.cpp, yarr/YarrInterpreter.cpp and runtime/RegExp*.cpp, and verify each allocation feeding those indices derives from m_offsetsSize rather than from numSubpatterns. Wider: repeat the exercise for other recent JSC layout extensions where a trailing region was appended to an existing count-derived array — modifier and v-flag additions, match-result rare data. Widest: the general "tail-extended structure with a legacy size formula" class, applying to versioned kernel/syscall structs, Vulkan pNext-style extension chains, and any wire format where a v2 field is appended after a v1 length calculation. Tell: two size expressions for the same buffer in the tree, one of which mentions the new feature's count and one of which does not.

• A simplified façade over an engine that re-implements the engine's setup sequence instead of sharing it, then drifts. The invariant is if two entry points must satisfy the same callee contract, the contract-satisfying code should exist once. Narrow: diff RegularExpression::match() against RegExp::finishCreation/matchInline line by line — beyond the sizing bug fixed here, note that match()'s offsetNoMatch initialization loop is still bounded by m_numSubpatterns + 1 while the buffer is now m_offsetsSize long, and WTF's VectorTraits leave POD elements uninitialized on grow(), so confirm by reading interpret() that every path writes each duplicate-group slot before reading it. Wider: audit the other thin JSC/WTF-level wrappers that embedders call directly (Yarr::checkSyntax paths, WTF-level string and URL matching helpers) for the same shape — a second, simpler caller of an engine whose primary caller has since gained extra setup steps. Widest: "lite façade drift", applying to libc wrappers over syscalls, high-level SDK clients over a protocol layer, ORM query builders over a driver. Tell: two call sites of the same low-level function where one performs strictly more setup than the other, and the extra setup was added after the simpler call site was written.

• Verify how far out-of-bounds writes stay hidden in code using Vector<T, N> with a large inline capacity, computing the container's reserved capacity rather than the requested length: below inline capacity the bound is exactly N, and after a heap growth expandCapacity() reserves max(requested, max(16, capacity() + capacity() / 4 + 1)), so a modest overrun past the requested size stays in the vector's own slack. Narrow: grep JavaScriptCore and WTF for Vector< declarations with an inline capacity of 16 or more that are subsequently grow()n to a computed size and then handed out as a raw pointer via mutableSpan().data() or data(); for each, work out whether the callee's maximum index can exceed the reserved capacity in either regime — the inline regime is where a write escapes onto the enclosing stack frame. Wider: the same blind spot exists for any small-buffer-optimised container passed as a raw pointer to a callee that indexes it independently (SmallVector-style types, fixed-size stack arrays with a separate length variable), and for growth policies that over-allocate and thereby mask small overruns from ASan. Tell: a raw pointer escaping a container whose reserved capacity and logical size can differ, with the callee indexing by values it derived elsewhere. Verifying this class statically is nontrivial — reproducing corruption generally requires driving the touched index past reserved capacity under ASan rather than just running the existing tests.

• Grep the YARR interpreter for reads of output[...] at duplicate-named-group indices that are not preceded by a write on every reachable path, since the fix widens the buffer without extending the wrapper's offsetNoMatch seeding loop past m_numSubpatterns + 1 and WTF leaves POD elements uninitialized on grow(). Start with ParenthesesDisjunctionContext's constructor and restoreOutput() in yarr/YarrInterpreter.cpp and walk outward to the term-dispatch code. Wider: the same "caller partially initializes a buffer the callee assumes is fully initialized" shape applies to any out-parameter array in JSC whose seeding loop and allocation length come from different expressions. Tell: an allocation length and an initialization loop bound in the same function written as two different expressions.