[6] Masonry fieldset legend passes isSubgrid() despite being excluded from parent grid

MediumWebCore renderingOOB

ca8c809

Medium — reachable from any page with zero JavaScript, and it produces an out-of-range grid line index inside layout state plus a reachable debug assertion. It stops short of higher because both consumers visible in the surrounding source clamp before indexing, so no read/write primitive could be substantiated.

CSS Grid Level 3 masonry lays items out along one axis into a fixed set of tracks on the other, and a nested grid can declare itself a subgrid — adopting its parent's track lines over the area it occupies rather than defining its own. Two objects meet in this diff: RenderGrid, the render-tree object for a grid container, and GridMasonryLayout, the helper that runs masonry placement over the grid axis and maintains a per-track running-position vector sized to the track count. Subgrid-ness presupposes that the child actually occupies a span in the parent's grid, and placement math presupposes that an item's requested span fits the tracks that exist.

The angle: a page with no script can declare a fieldset legend as a subgrid it was never placed into, or ask for a ten-track span in a one-track grid, and drive masonry placement into an out-of-range line index — a debug-build assertion and, in release, a GridArea whose containment rests on downstream clamps.

The commit message names both halves: RenderGrid::isSubgrid() does not check isExcludedFromNormalLayout(), so a fieldset legend styled with grid-template-rows: subgrid is excluded from the fieldset's grid item placement but still passes all subgrid checks — and when the legend's layout queries the parent grid for its span, the legend is not found, hitting ASSERT(m_gridItemArea.contains(item)) in debug builds and causing an unsigned underflow in gridAreaForIndefiniteGridAxisItem in release. The fix returns false from isSubgrid() for excluded elements and clamps the item span to the grid-axis track count per CSS Grid Level 3 §4.4.

Source/WebCore/rendering/RenderGrid.cpp

Source/WebCore/rendering/GridMasonryLayout.cpp

LayoutTests/fast/css-grid-layout/masonry-subgrid-excluded-legend-crash.html

LayoutTests/fast/css-grid-layout/masonry-span-exceeds-grid-axis-tracks-crash.html

Patch Details

RenderGrid::isSubgrid(Style::GridTrackSizingDirection) gains an early if (isExcludedFromNormalLayout()) return false; guard, inserted after the existing establishesIndependentFormattingContextIgnoringDisplayType(style()) check and before the style().gridTemplateList(direction).subgrid check. A renderer excluded from normal layout now reports false for subgrid-ness even when its computed style says grid-template-rows: subgrid.

GridMasonryLayout::gridAreaForIndefiniteGridAxisItem(const RenderBox&) clamps the resolved auto-placement span to the grid-axis track count via std::min<unsigned>(..., m_gridAxisTracksCount), where previously the raw spanSizeForAutoPlacedItem() result was used directly.

Collateral: two new layout tests plus their -expected.txt files — masonry-subgrid-excluded-legend-crash.html, whose universal selector applies display: grid-lanes; grid-template-rows: subgrid ... so a <fieldset><legend> chain hits the excluded-legend path, and masonry-span-exceeds-grid-axis-tracks-crash.html, a grid-lanes container with one row track containing a nested grid-lanes subgrid with an item declaring grid-row: span 10.

A participation predicate answered from declared configuration rather than actual membership, paired with an externally-supplied span consumed by index arithmetic without being bounded against capacity.

Background

CSS Grid subgrid. A nested grid container declared with grid-template-rows: subgrid (or -columns) does not define its own tracks in that axis; it adopts the parent grid's track lines over the span it occupies in the parent. This presupposes that the child actually occupies a span in the parent — that it is one of the parent's grid items.

Masonry / display: grid-lanes. CSS Grid Level 3 masonry lays items out along one axis (the masonry axis) into a fixed set of tracks on the other (the grid axis). GridMasonryLayout::m_gridAxisTracksCount is the number of grid-axis tracks; m_runningPositions is a Vector<LayoutUnit> sized to that count, holding the current fill height of each track.

Definite vs. indefinite grid-axis position. GridMasonryLayout::placeMasonryItems() routes each item to gridAreaForDefiniteGridAxisItem() if it has an explicit grid-axis placement, otherwise to gridAreaForIndefiniteGridAxisItem(), which auto-places it and derives a start line from the item's requested span and the available lines.

spanSizeForAutoPlacedItem(). A Style::GridPositionsResolver helper that resolves how many tracks an auto-placed item requests, e.g. from grid-row: span 10. Its value comes from author CSS.

isExcludedFromNormalLayout(). A RenderObject-level predicate marking renderers that the parent's normal layout does not lay out itself — the fieldset <legend> is the canonical case, since the fieldset renderer positions the legend specially rather than as an ordinary in-flow child.

Grid::m_gridItemArea. A HashMap<SingleThreadWeakRef<const RenderBox>, GridArea> mapping each placed item to its area. Grid::gridItemArea() asserts the key is present and otherwise returns HashMap::get()'s default — for GridArea that is the default constructor, which sets both rows and columns to GridSpan::indefiniteGridSpan().

GridSpan. A start/end line pair with distinct indefinite and translated-definite states; clamp() bounds it to a supplied track count.

Unsigned index arithmetic in C++. Subtracting a larger unsigned from a smaller one wraps modulo 2^N rather than producing a negative value, so a size comparison that would catch a negative result never fires.

Analysis

There are two independent missing invariants here, and the commit closes both — each test exercises one, which is the strongest evidence that they are distinct entry points rather than one bug seen twice.

The first is a predicate/participation mismatch. isSubgrid() answered from computed style alone plus the independent-formatting-context test. A fieldset <legend> is excluded from the fieldset's normal layout, taking a special layout path rather than being laid out as an ordinary in-flow child — yet pre-fix it could still answer yes, I am a subgrid of my parent. Subgrid-ness is only meaningful for a renderer that actually occupies a span in the parent's grid. What the excluded legend's subgrid answer then feeds into is an inference: no caller of RenderGrid::isSubgrid(direction) and no fieldset/legend placement code is in the supplied context. The plausible consumer shape is a span query through Grid::gridItemArea(), which is ASSERT(m_gridItemArea.contains(item)); return m_gridItemArea.get(item); — verified from the supplied Grid.cpp / Grid.h / GridArea.h. If the legend is absent from the map, that would trip the assertion in debug builds while release builds receive a default-constructed GridArea carrying indefinite spans, consumed as if it were a real placement.

The second is an unbounded author-controlled span. Independently of the first, gridAreaForIndefiniteGridAxisItem() took spanSizeForAutoPlacedItem() — driven directly by author CSS such as grid-row: span 10 — and used it without relating it to m_gridAxisTracksCount. The next line computes auto gridAxisLines = m_gridAxisTracksCount + 1;, and the placement math derives a start/end line from those two values. The body that consumes itemSpanLength is not in the supplied source, so the precise failure mode follows from the shape of the fix rather than being shown. What the clamp establishes is unambiguous: a span larger than the grid-axis track count previously reached placement math that cannot accommodate it. Given that the surrounding code works in unsigned, the likely mechanism is an unsigned subtraction of the span from the available line count wrapping to a very large value instead of going negative, producing an out-of-range grid line index that flows into GridArea construction.

Reachability is unambiguous and requires no JavaScript — both regression tests are pure HTML and CSS.

For masonry-span-exceeds-grid-axis-tracks-crash.html: .outer is display: grid-lanes with a single 10px row track; .inner is a nested grid-lanes whose grid-template-rows: subgrid makes it derive its grid-axis track count from the parent, giving one track; .item is a child of .inner, so the GridMasonryLayout instance placing it belongs to .inner, running with m_gridAxisTracksCount == 1 and m_runningPositions sized to 1. .item declares grid-row: span 10 with no definite grid-axis position, so gridAreaForIndefiniteGridAxisItem() runs; pre-fix, spanSizeForAutoPlacedItem() returned 10 while gridAxisLines was 2.

For masonry-subgrid-excluded-legend-crash.html: the universal selector gives every element display: grid-lanes; grid-template-rows: subgrid repeat(auto-fill, []) repeat(4, []), so the <legend> inside the <fieldset> computes as a subgrid; pre-fix isSubgrid() answered true because it consulted only style and the independent-formatting-context test, even though the legend is excluded from normal layout. Whether the resulting indefinite area then reaches the same span-vs-capacity arithmetic as the span 10 case could not be established from the supplied source; the two paths are separately patched, which is at least consistent with them being distinct.

Escalation beyond a stability issue is not supported by the supplied context. On the paths visible here the out-of-range line index is not consumed by an unguarded raw memory access: Grid::insert() clamps clampedArea.rows / .columns to m_maxRows / m_maxColumns when those are set, and GridMasonryLayout::updateRunningPositions() calls gridAxisSpan.clamp(m_runningPositions.size()) before the m_runningPositions[line] loads. If some other consumer of the resulting GridArea were to index a Vector without an equivalent clamp, a wrapped line value could produce a far out-of-bounds index; the supplied source does not show such a consumer, so this remains an unrealized projection. Grid::ensureGridSize() reached from insert() would also attempt m_grid.grow(maximumRowSize) on an unclamped huge row count, which could surface as an allocation failure rather than as controlled corruption.

This vulnerability weakens memory-safety-adjacent invariants inside the renderer's layout engine: that a renderer answering isSubgrid() is actually a participant in its parent's grid, and that a resolved item span is bounded by the grid-axis track count. What an attacker could plausibly obtain is an author-influenced out-of-range grid line index propagating through masonry placement — a debug-build assertion failure and, in release, a GridArea whose containment depends on downstream clamps rather than on the placement math being correct. Realistically this is a layout-correctness and renderer-stability issue reachable from any web page with no scripting required.

The two hunks illustrate a defence-in-depth pairing worth internalizing: one fix corrects the predicate so it reflects actual layout participation rather than declared style, the other hardens the arithmetic consumer so it clamps regardless of how it was reached. The predicate fix alone would have closed the legend path; the clamp alone would have closed the span 10 path; the author landed both with a regression test each. display: grid-lanes is young code and shows the characteristic signature of an area still stabilizing — updateRunningPositions() has a real clamp() guarded by an ASSERT that the clamp is unnecessary, and Grid::gridItemArea() asserts map membership while release builds silently fall back to a default-constructed indefinite GridArea. Every place where an ASSERT documents an invariant that release builds paper over with a legal-looking default value is a place where a predicate mismatch converts into silent bad state rather than a crash.

Audit directions

• Participation predicate answered from declared configuration while membership is populated elsewhere. The two can disagree, and every consumer that trusts the predicate then queries a set that does not contain the object. Narrow: audit the remaining RenderGrid predicates that gate participation-dependent work — isSubgridInParentDirection(), isSubgridRows(), isMasonry(), isExtrinsicallySized() in Source/WebCore/rendering/RenderGrid.cpp — and for each ask whether a renderer excluded from normal layout, out-of-flow positioned, or otherwise never inserted into Grid::m_gridItemArea can still answer true. Code-review tell: a predicate body that reads only style() and parent type with no check tied to whether the object was actually laid out or placed. Wider: the same class appears wherever WebCore has an isX() style-derived predicate paired with a separately-maintained container — flex item participation in RenderFlexibleBox, out-of-flow and RenderFragmentedFlow exclusion sets, and the various isExcludedFromNormalLayout() / establishesIndependentFormattingContext() consumers; the shape to notice in search results is a contains() or ASSERT(map.contains(...)) at the consumer end whose key is only ever inserted on a path the predicate does not check. Widest: declared-capability-vs-actual-registration covers ECS component queries vs. entity registration, DI container capability interfaces vs. registered lifetimes, Kubernetes label selectors vs. actual endpoint membership. Carry the invariant: a predicate that answers "am I a participant?" must be derived from the participation registry, not from the configuration that requests participation.

• Index arithmetic on an externally-supplied count measured against an internally-derived capacity. The capacity can legitimately be smaller than the request, and the arithmetic is done in an unsigned type. Narrow: grep Source/WebCore/rendering/GridMasonryLayout.cpp and GridTrackSizingAlgorithm.cpp for other uses of Style::GridPositionsResolver::spanSizeForAutoPlacedItem and of m_gridAxisTracksCount in subtraction or modulo position — insertIntoGridAndLayoutItem() already does gridAxisSpanFromArea(area).endLine() % m_gridAxisTracksCount, so any path that can reach it with m_gridAxisTracksCount == 0 or an out-of-range endLine is worth tracing. Code-review tell: an unsigned / size_t expression of the form capacity - authorControlledCount or capacity + 1 - span with no preceding std::min or clamp. Wider: the same class shows up throughout WebCore layout wherever a CSS-authored repeat/span/count meets a computed track or column capacity — repeat(auto-fill, ...) resolution, table column span resolution in RenderTableSection, multicol column-count math; look for any unsigned local initialized from a Style:: resolver and then used as the right operand of a subtraction. Widest: unsigned underflow at a capacity boundary holds in any language where the natural index type is unsigned — C/C++ size_t, Rust's debug-only overflow checks on usize, Go's uint slice indices. Carry the invariant: whenever an externally-supplied count is subtracted from an internally-derived capacity, either the comparison must precede the subtraction or the arithmetic must be done in a signed/saturating type.

• Release-build silent fallback behind a debug-only ASSERT. The fallback value is a legal-looking default that downstream code cannot distinguish from a real value. Investigate Grid::gridItemArea() in Source/WebCore/rendering/Grid.cpp — ASSERT(m_gridItemArea.contains(item)); return m_gridItemArea.get(item); returns a default-constructed GridArea whose spans are GridSpan::indefiniteGridSpan() — and enumerate its callers to determine which of them can tell an indefinite-by-default area from an intentionally indefinite one. Code-review tell: an ASSERT(container.contains(key)) immediately followed by an unchecked .get(key) on a value type with a meaningful default constructor. Wider: the same shape recurs across WebKit wherever a HashMap::get(), WeakPtr deref, or Vector index is preceded by an assertion rather than a check — and equally where a real bounds clamp() is preceded by an ASSERT claiming the clamp is unnecessary, as in updateRunningPositions(); search for ASSERT( lines whose condition is the negation of the very safety property the next statement enforces, since that pairing marks code where the author knew the invariant was fragile. Widest: assertion-as-documentation, default-as-behaviour applies to Python dict.get() with a default after an assert key in d, Java Optional.orElse after a precondition compiled out in production, Rust unwrap_or_default() guarded by a debug_assert!. Carry the invariant: if the release path can produce a value the debug path asserts is impossible, that value must be distinguishable at the call site or the accessor must be made fallible.