Avoid u16 underflow in xmtx advance for fonts with no long metrics - #136
Open
youdie006 wants to merge 1 commit into
Open
Avoid u16 underflow in xmtx advance for fonts with no long metrics#136youdie006 wants to merge 1 commit into
youdie006 wants to merge 1 commit into
Conversation
advance() computes the offset of the last long metric as (long_metric_count - 1) * 4. A font with numberOfHMetrics == 0 makes long_metric_count 0, so the subtraction underflows: it panics in debug builds (attempt to subtract with overflow) and wraps to 65535 in release builds, both from a font that FontRef accepts. Use saturating_sub(1) so a zero metric count clamps to offset 0 (which reads no metric and returns 0) instead of underflowing.
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
1 participant
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.
Fixes #133.
xmtx::advance()computes the offset of the last long metric with(long_metric_count - 1) * 4(src/internal/xmtx.rs:14).A font whose
numberOfHMetricsis 0 makeslong_metric_count == 0, solong_metric_count - 1underflows theu16: it panics in debug builds (attempt to subtract with overflowatxmtx.rs:14) and wraps to65535in release builds (an out-of-range read thatunwrap_or(0)then masks). The minimal 12-byte font in #133 is accepted byFontRef::from_index, soGlyphMetrics::advance_widthreaches this path.sb()is not affected — itselsebranch subtractslong_metric_countfromglyph_id(which is>= long_metric_countthere), so it cannot underflow.Fix: use
long_metric_count.saturating_sub(1), so a zero metric count clamps to offset 0 (which reads no metric and returns 0) instead of underflowing.Test: added a unit test in
xmtx.rscallingadvance(&[], 0, 0, _). Verified red before the fix (panics withattempt to subtract with overflow) and green after.cargo test(1 + 17 passed),cargo fmt --check, andcargo clippy(no new warnings) pass locally.Disclosure: developed with the assistance of Claude Code (AI); reviewed and verified by me.