Cap total component visits in glyf/gvar composite outlining - #224
Open
scadastrangelove wants to merge 1 commit into
Open
Cap total component visits in glyf/gvar composite outlining#224scadastrangelove wants to merge 1 commit into
scadastrangelove wants to merge 1 commit into
Conversation
MAX_COMPONENTS bounds the depth of a composite-glyph chain, but nothing bounds how many times a shared component-glyph subtree can be revisited via sibling components at the same level. A composite glyph whose components all reference one shared child glyph forces branching^depth calls to outline_impl while the depth cap is never reached, needing only depth+1 distinct glyphs to construct. The identical pattern exists in gvar::outline_var_impl. Adds glyf::MAX_COMPONENT_VISITS = 100_000 and threads a total-visit budget through both outline_impl and outline_var_impl, independent of depth or branching factor. Discovered by the rust-in-peace security pipeline (https://github.com/scadastrangelove/rust-in-peace/).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #220.
Adds
glyf::MAX_COMPONENT_VISITS: u32 = 100_000and threads abudget: &mut u32parameterthrough
outline_impl(decremented once per call,checked_sub—Noneon exhaustion abortsthe whole traversal same as the existing depth-cap
None) and throughgvar::outline_var_impl(same mechanism, reusingglyf::MAX_COMPONENT_VISITS). Both publiccall sites (
glyf::Table::outline,gvar::Table::outline) initialize a fresh budget pertop-level call.
This is deliberately a total node-visit budget, not a depth or branching-factor cap — it's
the shared-subtree reuse that defeats
MAX_COMPONENTS, not depth itself, so the fix needs tobound total work rather than tree shape.
Testing
cargo test --all-features --release— 148/149 (pre-existing unrelatedfailure, see Guard CFF2 BLEND operator against empty argument stack #222 for detail).
(previously didn't complete in 20s) now completes in ~2.5ms. Both return the same outcome as
before (this glyph's outline is empty either way) — only the cost changed.
8.66ms/6253 glyphs): the capped worst case is now well within the same order of magnitude as
legitimate composite-glyph parsing, not 13,600× beyond it.
Discovered by the rust-in-peace security pipeline.