← All reports

Integer underflow in the OpenType VORG table size computation

Component: WebCore Font Parsing | 78b8d62

OpenType tables are parsed directly from font bytes supplied by @font-face resources, which can be arbitrary and attacker-controlled. VORGTable models the OpenType VORG (Vertical Origin) table using a common C pattern: a fixed-size struct with a trailing one-element array, vertOriginYMetrics[1], where the real element count comes from the font. requiredSize() computes how many bytes the table actually needs so a caller can bounds-check before reading the array — arithmetic that is easy to get subtly wrong on attacker-controlled counts.

This commit replaces sizeof(*this) + sizeof(VertOriginYMetrics) * (numVertOriginYMetrics - 1) in VORGTable::requiredSize() (OpenTypeVerticalData.cpp) with an offsetof-based formula that does not underflow when numVertOriginYMetrics is 0. It also adds a regression test using a font whose GSUB FeatureList table is truncated mid-record.

The old formula underflowed at a count of zero, but the wraparound happened to cancel back to the correct offset for this specific struct, which has no trailing padding. So this is hardening rather than the closure of an active bounds-check bypass — it removes a fragile, coincidentally-correct idiom whose correctness depended on a layout property nobody had written down. The added test does lock in real out-of-bounds-read protection, but for a separate, already-landed fix to a findFeature OOB read in GSUB parsing.

Narrow: sweep the other OpenType table models for the same sizeof(*this) + sizeof(Element) * (count - 1) idiom, since each instance is correct only by accident of that particular struct's padding — and the match tell is a - 1 inside a size computation whose operand comes from font bytes. Wider: the portable pattern is trailing-array size arithmetic where an unsigned count of zero drives a subtraction below zero; it appears wherever a flexible-array-member struct is sized from external data, not just in font parsing, and the general hunt is for offsetof being the correct form that an older codebase spelled as sizeof minus one element. Widest: coincidental correctness is its own audit category — any expression that is right only because two errors cancel will stop being right the moment someone adds a field, so the review tell is arithmetic whose correctness argument requires reasoning about struct padding at all.