[6] ANGLE IndexRange: stored inclusive count replaced by a derived one

LowANGLE — gl::IndexRangeIntegerOverflow

56c55dd

Low — and specifically not a live bypass. The pre-image stores the inclusive cardinality in uint64_t, so the maximal span is reported correctly and no bounds check is skipped. What the patch removes is the structural hazard: a count duplicated alongside its endpoints, with "empty" encoded as a value that same arithmetic could produce.

An inclusive range over an N-bit integer type needs N+1 bits to express its own cardinality — [0, 0xFFFFFFFF] holds 0x100000000 elements. That arithmetic fact sits directly under WebGL's indexed-draw bounds validation, which must guarantee that a glDrawElements call from web content can never reference a vertex outside the storage bound to an enabled attribute. ANGLE's gl::IndexRange is the value type carrying that span: produced by scanning the untrusted element-array buffer, cached, and compared against each attribute's element limit. Its isEmpty() predicate is what tells callers "there is nothing here to validate".

The angle: defensive only — the maximal index span was already reported correctly, but the empty encoding can no longer be synthesised by cardinality arithmetic, and the added tests pin GL_INVALID_OPERATION for {0, 0xFFFFFFFF} across all three index widths.

IndexRange would mark up index range with uint32_t start, uint32_t count which can not easily represent range [0, 0xFFFFFFFF]. Switch to start, end markup, with start > end marking empty range.

Source/ThirdParty/ANGLE/src/common/mathutil.h

Source/ThirdParty/ANGLE/src/tests/gl_tests/WebGLCompatibilityTest.cpp

Source/ThirdParty/ANGLE/src/common/utilities_unittest.cpp

Patch Details

The change reworks gl::IndexRange from a (start, end, stored-count) representation to a pure (start, end) one, then extends the test coverage on both the unit and conformance sides.

On the type itself: the constructor no longer computes and stores mCount, only mStart/mEnd with ASSERT(mStart <= mEnd). isEmpty() changes from mCount == 0 to mStart > mEnd, and the default member initializers change from mStart{0}, mEnd{0}, mCount{0} to mStart{1}, mEnd{0}, so a default-constructed range is a canonical empty range lying outside the endpoint ordering the constructor's contract permits. vertexCount() is re-expressed as a derived size_t computation static_cast<size_t>(mEnd) - mStart + 1u, with an explicit comment documenting that the inclusive maximal range [0, 0xFFFFFFFF] has 0x100000000 elements and therefore does not fit in 32 bits, plus a note that unsigned underflow deliberately produces 0 for the empty range. The hand-written operator== — which compared only vertexCount() and, for non-empty ranges, start() — is deleted and replaced by a defaulted member-wise friend bool operator==.

On the test side: utilities_unittest.cpp gains boundary-value tests for ComputeIndexRange with UnsignedInt indices including 0xffffffff, plus direct isEmpty()/vertexCount() assertions at the 32-bit boundary, and the Xcode project adds the file to the unit test target. WebGLCompatibilityTest.cpp is extended so the tested program has a buffer-backed vertex attribute, then draws with index data {0, 0xFFFFFFFF} expecting GL_INVALID_OPERATION for GL_UNSIGNED_INT/GL_UNSIGNED_SHORT/GL_UNSIGNED_BYTE element types, and with {0, 1} expecting GL_NO_ERROR.

Duplicating a range's cardinality as stored state alongside its endpoints while encoding 'empty' as a value the cardinality arithmetic can itself produce.

Background

Where this lives. ANGLE is the OpenGL ES / EGL translation layer backing WebGL in WebKit, running in the GPU process, so its validation layer sits between web-controlled draw parameters and the platform graphics backend.

Indexed drawing. glDrawElements(mode, count, type, offset) reads count indices of type (GL_UNSIGNED_BYTE/SHORT/INT) from the buffer bound to GL_ELEMENT_ARRAY_BUFFER starting at offset, and for each index fetches the corresponding element from every enabled vertex attribute array.

gl::IndexRange. An ANGLE value type describing the inclusive [start, end] span of vertex indices referenced by an indexed draw call, together with a vertexCount() giving the number of vertices the span covers. ComputeTypedIndexRange in common/utilities.cpp scans count indices of a given integer width, tracks minIndex/maxIndex, and returns gl::IndexRange(minIndex, maxIndex); when it sees no vertices it returns a default-constructed range.

Primitive restart. A mode in which one reserved index value (std::numeric_limits<IndexType>::max()) means "start a new primitive" instead of "fetch this vertex". When enabled, ComputeTypedIndexRange skips that value when computing min/max, which is why the same index buffer yields different ranges with the flag on and off.

Index range caching. gl::IndexRangeCache (a std::map<IndexRangeKey, IndexRange>) and gl::IndexRangeInlineCache memoise the computed range keyed on element type, offset, count, and primitive-restart flag, so the expensive scan runs once per index-buffer region. Lookups compare keys via IndexRangeKey::operator== / operator< and copy the stored IndexRange payload out.

Vertex attribute element limits. VertexAttribute::getCachedElementLimit(), maintained by VertexArray::updateCachedElementLimit(), records how many elements an enabled attribute can supply given its bound buffer's size, stride, and offset. WebGL, unlike desktop GL, requires draws referencing indices beyond that limit to be rejected with GL_INVALID_OPERATION rather than left as undefined behaviour.

ANGLE's ASSERT. The macro from common/debug.h is active only in builds with assertions enabled and compiles out otherwise, so an ASSERT-expressed precondition documents a contract for debug builds rather than enforcing it at runtime in release.

Inclusive versus half-open ranges. For an inclusive range over uint32_t, cardinality is end - start + 1, whose maximum value (0x100000000, for [0, 0xFFFFFFFF]) requires 33 bits. size_t is 64 bits on LP64 targets and 32 bits on ILP32 targets.

Analysis

Start with what is not wrong. In the pre-image this diff replaces, mCount is a uint64_t computed as static_cast<uint64_t>(end - start) + 1. That arithmetic is exact for start <= end — the uint32_t subtraction cannot wrap under the constructor's own precondition, and the widening cast happens before the + 1 — so IndexRange(0, 0xFFFFFFFF).vertexCount() is 0x100000000 and isEmpty() correctly returns false. No 32-bit wrap-to-empty exists in the code this patch replaces.

The defects in the pre-image are structural: (a) the cardinality is duplicated state that every construction site must keep in agreement with [mStart, mEnd]; (b) emptiness is encoded as mCount == 0, a value inside the arithmetic's own output domain rather than an out-of-domain endpoint pair; and (c) the default-constructed range mStart{0}, mEnd{0} is structurally identical to the valid single-vertex range [0, 0] in its endpoint members — the two were distinguishable only because mCount carried the extra bit, so any consumer reading mStart/mEnd alone, or any future change narrowing the count, would collapse them.

What the fix forecloses is the hazard those defects leave open. ComputeTypedIndexRange() scans the element-array buffer and returns gl::IndexRange(minIndex, maxIndex); for UnsignedInt content of {0, 0xFFFFFFFF} with primitive restart disabled this is the maximal inclusive span, whose cardinality needs 33 bits. Any representation materialising that cardinality in a 32-bit quantity would produce 0 — which, under an isEmpty() { return mCount == 0; } predicate, would decode the widest possible index range as no vertices at all. The patch removes that structurally: vertexCount() is now derived so no consumer can observe a count disagreeing with the endpoints, emptiness moves to mStart > mEnd, and the default initialisers put the empty encoding outside the ordering the constructor's contract permits. That contract is expressed by ASSERT(mStart <= mEnd), which compiles out when assertions are disabled — so the separation is convention plus a debug tripwire, not a construction-time guarantee, and the same caveat applies to the ASSERT(!isEmpty()) guards in start()/end().

The operator== replacement is part of the same de-duplication: the old comparator projected the type onto (vertexCount(), start()) and treated all zero-count ranges as equal regardless of endpoint members, so it compared a derived view rather than the state. Note this operator is not what decides index-range cache hits — per libANGLE/IndexRangeCache.h, IndexRangeInlineCache::get() compares IndexRangeKey::operator== and IndexRangeCache's std::map orders on IndexRangeKey::operator<; the IndexRange is only the payload copied out. IndexRange::operator== therefore governs value comparisons such as the added unit-test assertions, not lookup correctness.

The new conformance case pins the end-to-end behaviour the representation must support: (1) a program with an a_Position attribute, glEnableVertexAttribArray(posLocation), and glVertexAttribPointer(posLocation, 4, GL_FLOAT, GL_FALSE, 0, nullptr) against a GL_ARRAY_BUFFER holding 12 floats — three vec4 vertices, so the attribute can supply three elements; (2) {0, 0xFFFFFFFF} uploaded as GL_UNSIGNED_INT index data with primitive restart disabled; (3) glDrawElements(GL_LINES, 2, GL_UNSIGNED_INT, 0), asserted to fail with GL_INVALID_OPERATION. The GL_UNSIGNED_SHORT-at-offset-2 and GL_UNSIGNED_BYTE-at-offset-3 variants reinterpret the same bytes as {0, 0xFFFF} and {0, 0xFF}, which also exceed the three-element attribute and must be rejected.

The projection the patch defends against is conditional and not exhibited here: if any representation of this type materialised the inclusive cardinality at 32-bit width — as the derived size_t expression would on an ILP32 target — the maximal span would evaluate to a count of 0, and any consumer keyed on isEmpty() or vertexCount() == 0 could then take a "nothing to validate" path for the widest possible index range. Realising that would additionally require such a narrowed configuration to exist in a shipping build, and the element-limit comparison against getCachedElementLimit() to be guarded by that predicate; the supplied validationES.cpp excerpt is truncated and contains no IndexRange consumer, so neither is established. Had a misreport been reachable, the plausible attacker gain would have been a relative out-of-bounds vertex fetch at a page-chosen offset scaled by the attribute stride, whose contents could flow into shader inputs and might be recoverable via rendering plus readPixels — an information-disclosure or crash outcome in the GPU process, not a write, since gl::IndexRange gates a read-side magnitude comparison.

The boundary this value type sits on is WebGL's mandatory indexed-draw bounds validation, since the platform backend and driver are not a trust boundary for web content. The security model assumption is that the computed index range faithfully describes the untrusted index buffer, including at the extremes of the 32-bit index space, and that "empty" means "no vertices" rather than "cardinality wrapped". This commit is best read as hardening rather than as closing a live bypass.

Two design smells compound in the pre-image, and both are worth carrying forward even though neither is live here. First, an inclusive range needs one more bit of cardinality than its endpoint type provides, so any inclusive-range type materialising its own length at the endpoint's width is one boundary value away from wrapping; the patch's own comment shows the author reasoning about exactly this. Second, and more dangerous, is sentinel aliasing: mCount == 0 served double duty as a real cardinality and as the "nothing here" marker, so a wrapped count for the most out-of-bounds range would decode as the least dangerous state. Wherever validation code takes an if (range.isEmpty()) skip_the_check; shortcut, a sentinel that arithmetic can synthesise turns that shortcut into a candidate bypass. One caveat the patch introduces on a different axis: vertexCount() now returns size_t rather than uint64_t, so on an ILP32 build the maximal range's cardinality would evaluate to 0 while isEmpty() reports false — the derived-count design is only equivalent to the old stored uint64_t on LP64 targets, which is worth confirming for every ANGLE build configuration consuming this value.

Audit directions

• An inclusive range that materialises its own cardinality at a width no greater than its endpoints. The invariant is the cardinality of an inclusive [lo, hi] over an N-bit type needs N+1 bits, therefore it must be derived into a provably wider type, never stored or computed at endpoint width. Narrow: grep Source/ThirdParty/ANGLE/src/common and src/libANGLE for structs keeping a start/end pair alongside a count/size/length member, and for + 1 in constructors or accessors — start with the consumers of gl::IndexRange (ComputeTypedIndexRange, IndexRangeCache::invalidateRange, VertexArray::updateCachedElementLimit), and specifically confirm for every supported build configuration that size_t is 64-bit where the new vertexCount() expression is evaluated. Wider: the same class shows up wherever half-open and inclusive conventions meet in one type — buffer sub-range invalidation, texture level/layer spans, pixel-rect clipping in libANGLE/angletypes.h. In code review, the shape to notice is any end - start + 1 or last - first + 1 assigned into a variable of the same width as end, or into a platform-width type such as size_t/unsigned. Widest: this applies to any codebase with closed intervals over fixed-width integers — kernel VMA/extent structures, database key-range planners, Rust's RangeInclusive::len, LLVM's ConstantRange. If a type exposes both endpoints and a length, ask what the length is when the endpoints are the extremes of their type, and whether that value is representable in the length's type on every target.

• A sentinel value drawn from inside the valid value space, so an arithmetic error can synthesise it. The invariant is the "no data" encoding should be unreachable by any well-formed constructor, and where it rests only on a debug assertion, that gap is itself the finding. Narrow: audit ANGLE value types whose emptiness/validity predicate tests a numeric field against 0 or against std::numeric_limits<T>::max() — gl::IndexRange::isEmpty() before this patch, VertexAttribute::kIntegerOverflow in libANGLE/VertexAttribute.h, GL_INVALID_INDEX returns from gl::ParseArrayIndex, and the primitive-restart index in ComputeTypedIndexRange; for each, ask whether an overflowing or truncating path can land on the sentinel, and whether the guard separating sentinel from payload is a real branch or an ASSERT that vanishes in release. Wider: the same class appears in every magic-value-means-absent encoding reachable from untrusted input — size_t(-1) as not-found, -1 as an unset handle, 0 as both a valid offset and a null marker; the tell is a comparison against a literal or a numeric_limits extreme that is also a legal payload value. Prefer designs making absence a separate state — optional, tagged union, or an out-of-domain endpoint pair as this patch chose. Widest: the in-band-sentinel class holds anywhere a parser, allocator, or validator encodes absence inside the data domain — C's strtoul ULONG_MAX, mmap's MAP_FAILED, HTTP length fields, protobuf default-zero fields. If an attacker can steer arithmetic onto the sentinel, they select the code path guarded by "nothing to do here".

• A validation shortcut keyed on a derived predicate rather than on the raw data. The invariant is an early-out in a security check must be justified by a value the attacker cannot steer. Narrow: first establish whether such an early-out exists at all on ANGLE's indexed-draw path — trace the consumers of IndexRange::vertexCount(), end(), and isEmpty() in libANGLE/validationES.cpp and libANGLE/VertexArray.cpp, and check whether the value is narrowed into GLuint, GLint, or uint32_t before being compared against getCachedElementLimit(). Wider: the same shape recurs in any validator with a fast path — instance-count and divisor computations in ComputeVertexBindingElementCount, transform-feedback capacity checks, texture upload size checks; the tell is a guard of the form if (computedSize == 0) return true; sitting in front of the real bounds comparison. Widest: this is the general attacker-influenced-predicate-gates-the-check class, applicable to any bounds checker with a zero-length fast path — Chromium's Mojo message validators, image decoders skipping work on zero dimensions, kernel copy routines short-circuiting on len == 0. Whenever a check is skipped because a computed size is zero, ask which inputs make that size zero and whether any of them describe an enormous rather than an empty object.

• A hand-written equality operator comparing a projection of a type's state instead of its state. The invariant is equality on a value type used for memoisation or validation must be injective over the fields that affect behaviour. Narrow: check ANGLE's cache-adjacent value types for bespoke operator== implementations ignoring members — this patch replaced one on gl::IndexRange that folded every zero-count range into one class; verify IndexRangeKey::operator==/operator< in libANGLE/IndexRangeCache.h, which are what actually decide IndexRangeInlineCache::get() hits and std::map ordering, and the VertexAttribCurrentValueData comparators. Wider: the same class covers any custom hash/equality pair used for memoisation of a validation result — program/shader cache keys in MemoryProgramCache, blob cache keys in BlobCache — where a comparator omitting a field lets one entry answer for a differently-shaped query; the tell is an operator== touching fewer members than the struct declares, or a hash function and an equality function disagreeing about which fields matter. Widest: the memoisation-key-coarser-than-the-computation class applies to any cached security decision — HTTP cache keys omitting a Vary header, permission caches keyed on principal but not resource, JIT inline caches keyed on a partial shape. For any cached or compared validation result, ask which two different inputs compare equal, and whether one of them is safe while the other is not.