Unbounded recursion in skrifa VARC condition evaluation (eval_condition) -> stack overflow on untrusted fonts

[scadastrangelove]

Summary

skrifa's VARC condition evaluator recurses over the Condition expression
tree with no depth limit and no recursion guard. The condition tree is
fully attacker-controlled font data, so a crafted VARC table whose condition
tree nests Format3And / Format4Or / Format5Negate deeply drives
eval_condition into unbounded recursion until the stack overflows. A Rust
stack overflow is an uncatchable process abort (SIGABRT, not a panic —
catch_unwind cannot recover it), so any caller that outlines a glyph from an
untrusted font can be taken down.

• Crate: skrifa 0.45.1 (uses read-fonts 0.42.1)
• Repo: googlefonts/fontations, main @ f95a2db5b2367ddbe752884c39e13b62a2ea39bd
• Class: unbounded recursion / stack exhaustion (CWE-674), denial of service
• Memory safety: not affected — no OOB, no corruption; the process aborts
  cleanly on the guard page.

This is the same bug class as the recently fixed
#1993 (type1: remove unbounded recursion),
just in the VARC condition evaluator.

Location

skrifa/src/outline/varc/mod.rs, fn eval_condition (around line 646). The
three compound-condition arms recurse on nested child conditions with no depth
parameter:

Condition::Format3And(condition) => {
    for nested in condition.conditions().iter() {
        let nested = nested?;
        if !Self::eval_condition(&nested, coords, var_store, regions, scalar_cache, scratch)? {
            return Ok(false);
        }
    }
    Ok(true)
}
Condition::Format4Or(condition) => {
    for nested in condition.conditions().iter() {
        let nested = nested?;
        if Self::eval_condition(&nested, coords, var_store, regions, scalar_cache, scratch)? {
            return Ok(true);
        }
    }
    Ok(false)
}
Condition::Format5Negate(condition) => {
    let nested = condition.condition()?;
    Ok(!Self::eval_condition(&nested, coords, var_store, regions, scalar_cache, scratch)?)
}

Note the existing DrawError::RecursionLimitExceeded guard in this file
(draw_glyph, ~line 319, stack.len() >= GLYF_COMPOSITE_RECURSION_LIMIT)
bounds VARC component recursion — a different loop. Condition-expression
nesting is not covered by it, nor by any other limit.

Impact / reachability (honest framing)

Robustness of skrifa's public outline API on untrusted font bytes, not a
browser-reachable bug. Reaching path:

OutlineGlyph::draw()  ->  varc::Outlines::draw()  ->  draw_glyph()
                      ->  component_condition_met()  ->  eval_condition()   [overflow]

• Any code that calls font.outline_glyphs().get(gid)?.draw(..) (or
  OutlineGlyphCollection / metrics paths that evaluate VARC components) on a
  font it did not author is exposed: font-inspection / conversion tools, server
  side rendering / thumbnailing, test harnesses, fuzzers, and rendering of
  arbitrary system-installed fonts.
• Not reachable through Chromium's web font path: OTS sanitization strips
  the VARC table before skrifa sees it. This is a system-font / untrusted-
  input robustness issue, not a web-platform one.

Severity: Medium — trivially craftable, deterministic, uncatchable
process abort (DoS), but no memory corruption and not web-reachable.

Reproduction

The smallest deep tree is a linear chain of Format5Negate conditions. Each
ConditionFormat5 table is format: u16 + condition_offset: Offset24
(relative to the table), i.e. the 5-byte pattern 00 05 | 00 00 05 — format
5, offset 5 — which points at the next table 5 bytes on. Repeat it N times and
you have an N-deep condition tree in 5 * N bytes.

A unit-level reproduction (the vulnerable eval_condition is a private fn; it
is reached in production only via the public draw path above) — build the
chain, parse the root with the public read-fonts API, and evaluate it on a
small-stack thread so the overflow is fast and deterministic:

use read_fonts::{FontData, FontRead, TableProvider, FontRef};
use read_fonts::tables::layout::Condition;

// N Format5Negate tables, each pointing 5 bytes forward to the next.
const CHAIN_LEN: usize = 1_000_000;
let mut bytes = Vec::with_capacity(CHAIN_LEN * 5);
for _ in 0..CHAIN_LEN {
    bytes.extend_from_slice(&[0x00, 0x05, 0x00, 0x00, 0x05]);
}
let cond = Condition::read(FontData::new(&bytes)).unwrap();
// ... drive Outlines::eval_condition(&cond, ..) on a 1 MiB-stack thread ...

Observed on current main:

running 1 test
thread '<unknown>' has overflowed its stack
fatal runtime error: stack overflow, aborting
... (signal: 6, SIGABRT: process abort signal)

Suggested fix

Thread a depth counter through eval_condition and return
DrawError::RecursionLimitExceeded once it exceeds a bound, mirroring the
existing component-recursion guard in this file. Reusing the crate's existing
GLYF_COMPOSITE_RECURSION_LIMIT (32) keeps it consistent with the sibling
VARC component guard and is already ~6–8× deeper than any legitimate condition
tree (real ConditionSets are a flat AND of a handful of axis ranges, depth
≤ ~4). A full patch (fix + regression tests) is attached in the accompanying
PR; the deeply-nested chain then returns Err(RecursionLimitExceeded) and the
crate test suite (cargo test -p skrifa) stays green, including the existing
real-VARC-font drawing tests.

---

Found with rust-in-peace (https://github.com/scadastrangelove/rust-in-peace).