[glyf] Charge point loading to the work budget before doing the work (#6256)
The glyf outline budget was only charged after a glyph loaded
successfully: Glyph::get_points() spent 16 units per node up front, and
glyf_accelerator_t::get_points() charged one unit per point only after
the whole tree came back. A glyph that loaded ~175K points and then
tripped a structural cap (HB_GLYF_MAX_POINTS, edge count) paid for the
nodes but not the points. Driven from a COLR paint walk through
hb_font_get_glyph_extents(), one such glyph was loaded and thrown away
over 8000 times per session within the budget, ~1.4G point loads.
Charge at the point where the work is attempted instead:
- SimpleGlyph::get_contour_points() takes the budget and charges
num_points before allocating, clearing, and reading, so truncated or
later-rejected glyphs pay for the points they allocate.
- Composite levels charge the component subtree size before
transform_points(), since each nesting level transforms every point
below it and that multiplication was previously invisible to the
budget.
The terminal per-point charge for path emission is unchanged.
Local hb-vector-fuzzer run on the testcase: 4.3s -> 0.12s. Full test
suite passes. The testcase is added as a fuzz seed.
Fixes: https://oss-fuzz.com/testcase-detail/5714817420427264
Claude-Session: https://claude.ai/code/session_013ksLGe7wUVwbuANqR56YaN
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
diff --git a/src/OT/glyf/Glyph.hh b/src/OT/glyf/Glyph.hh
index 1376d53..3a1e620 100644
--- a/src/OT/glyf/Glyph.hh
+++ b/src/OT/glyf/Glyph.hh
@@ -344,7 +344,7 @@
head_maxp_info->maxContours = hb_max (head_maxp_info->maxContours, (unsigned) header->numberOfContours);
if (depth > 0 && composite_contours)
*composite_contours += (unsigned) header->numberOfContours;
- if (unlikely (!SimpleGlyph (*header, bytes).get_contour_points (all_points, phantom_only)))
+ if (unlikely (!SimpleGlyph (*header, bytes).get_contour_points (all_points, phantom_only, budget)))
return false;
break;
case COMPOSITE:
@@ -476,6 +476,15 @@
for (unsigned int i = 0; i < PHANTOM_COUNT; i++)
phantoms[i] = comp_points[comp_points.length - PHANTOM_COUNT + i];
+ /* Each composite level transforms the whole subtree below it;
+ * charge that work here so deep nesting is bounded by the budget
+ * rather than by the per-draw point cap alone. */
+ if (unlikely (!hb_budget_spend (*budget, HB_BUDGET_1, comp_points.length)))
+ {
+ points.resize (old_length);
+ return false;
+ }
+
if (comp_points) // Empty in case of phantom_only
{
float matrix[4];
diff --git a/src/OT/glyf/SimpleGlyph.hh b/src/OT/glyf/SimpleGlyph.hh
index 74c6509..a61b286 100644
--- a/src/OT/glyf/SimpleGlyph.hh
+++ b/src/OT/glyf/SimpleGlyph.hh
@@ -178,7 +178,8 @@
}
bool get_contour_points (contour_point_vector_t &points /* OUT */,
- bool phantom_only = false) const
+ bool phantom_only = false,
+ int64_t *budget = nullptr) const
{
const HBUINT16 *endPtsOfContours = &StructAfter<HBUINT16> (header);
int num_contours = header.numberOfContours;
@@ -188,6 +189,10 @@
unsigned int num_points = endPtsOfContours[num_contours - 1] + 1;
if (unlikely (num_points < (unsigned) num_contours)) return false;
+ /* Charge before allocating and reading, so that glyphs that fail
+ * later (truncated data, composite caps) still pay for this work. */
+ if (budget && unlikely (!hb_budget_spend (*budget, HB_BUDGET_1, num_points))) return false;
+
unsigned old_length = points.length;
points.alloc (points.length + num_points + 4); // Allocate for phantom points, to avoid a possible copy
if (unlikely (!points.resize_dirty (points.length + num_points))) return false;
diff --git a/test/fuzzing/fonts/clusterfuzz-testcase-minimized-hb-vector-fuzzer-5714817420427264 b/test/fuzzing/fonts/clusterfuzz-testcase-minimized-hb-vector-fuzzer-5714817420427264
new file mode 100644
index 0000000..72d1c74
--- /dev/null
+++ b/test/fuzzing/fonts/clusterfuzz-testcase-minimized-hb-vector-fuzzer-5714817420427264
Binary files differ
