From 52d8a0ace211d77a8b0c3a37a1fb689552de1c1e Mon Sep 17 00:00:00 2001 From: Chad Brokaw Date: Mon, 21 Oct 2024 09:31:36 -0400 Subject: [PATCH 1/2] Avoid various overflows in CFF Add targeted `wrapping_add` and `wrapping_sub` calls to avoid panics on arithmetic overflows for the specific case in #1193 --- read-fonts/src/tables/postscript/stack.rs | 8 +++++++- skrifa/src/outline/cff/hint.rs | 11 +++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/read-fonts/src/tables/postscript/stack.rs b/read-fonts/src/tables/postscript/stack.rs index 78e5dbfc7..157a25453 100644 --- a/read-fonts/src/tables/postscript/stack.rs +++ b/read-fonts/src/tables/postscript/stack.rs @@ -212,7 +212,7 @@ impl Stack { .iter_mut() .zip(&mut self.value_is_fixed) { - sum += if *is_fixed { + let fixed_value = if *is_fixed { // FreeType reads delta values using cff_parse_num which // which truncates the fractional parts of 16.16 values // See delta parsing: @@ -224,6 +224,12 @@ impl Stack { } else { Fixed::from_i32(*value) }; + // See + // The "DIN Alternate" font contains incorrect blue values + // that cause an overflow in this computation. FreeType does + // not use checked arithmetic so we need to explicitly use + // wrapping behavior to produce matching outlines. + sum = sum.wrapping_add(fixed_value); *value = sum.to_bits(); *is_fixed = true; } diff --git a/skrifa/src/outline/cff/hint.rs b/skrifa/src/outline/cff/hint.rs index 92174b205..33758e670 100644 --- a/skrifa/src/outline/cff/hint.rs +++ b/skrifa/src/outline/cff/hint.rs @@ -267,14 +267,17 @@ impl HintState { /// /// See fn capture(&self, bottom_edge: &mut Hint, top_edge: &mut Hint) -> bool { + // We use some wrapping arithmetic on this value below to avoid panics + // on overflow and match FreeType's behavior + // See let fuzz = self.blue_fuzz; let mut captured = false; let mut adjustment = Fixed::ZERO; for zone in self.zones() { if zone.is_bottom && bottom_edge.is_bottom() - && (zone.cs_bottom_edge - fuzz) <= bottom_edge.cs_coord - && bottom_edge.cs_coord <= (zone.cs_top_edge + fuzz) + && zone.cs_bottom_edge.wrapping_sub(fuzz) <= bottom_edge.cs_coord + && bottom_edge.cs_coord <= zone.cs_top_edge.wrapping_add(fuzz) { // Bottom edge captured by bottom zone. adjustment = if self.suppress_overshoot { @@ -294,8 +297,8 @@ impl HintState { } if !zone.is_bottom && top_edge.is_top() - && (zone.cs_bottom_edge - fuzz) <= top_edge.cs_coord - && top_edge.cs_coord <= (zone.cs_top_edge + fuzz) + && zone.cs_bottom_edge.wrapping_sub(fuzz) <= top_edge.cs_coord + && top_edge.cs_coord <= zone.cs_top_edge.wrapping_add(fuzz) { // Top edge captured by top zone. adjustment = if self.suppress_overshoot { From 77d88ae062126e6c0812aa0d769c0de87366fe20 Mon Sep 17 00:00:00 2001 From: Chad Brokaw Date: Mon, 21 Oct 2024 10:00:52 -0400 Subject: [PATCH 2/2] add link to FT delta code; more wrapping_sub --- read-fonts/src/tables/postscript/stack.rs | 3 +++ skrifa/src/outline/cff/hint.rs | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/read-fonts/src/tables/postscript/stack.rs b/read-fonts/src/tables/postscript/stack.rs index 157a25453..b8eca2cb5 100644 --- a/read-fonts/src/tables/postscript/stack.rs +++ b/read-fonts/src/tables/postscript/stack.rs @@ -204,6 +204,9 @@ impl Stack { /// "The second and subsequent numbers in a delta are encoded as the /// difference between successive values." /// + /// Roughly equivalent to the FreeType logic at + /// + /// /// See pub fn apply_delta_prefix_sum(&mut self) { if self.top > 1 { diff --git a/skrifa/src/outline/cff/hint.rs b/skrifa/src/outline/cff/hint.rs index 33758e670..61035eec8 100644 --- a/skrifa/src/outline/cff/hint.rs +++ b/skrifa/src/outline/cff/hint.rs @@ -282,7 +282,7 @@ impl HintState { // Bottom edge captured by bottom zone. adjustment = if self.suppress_overshoot { zone.ds_flat_edge - } else if zone.cs_top_edge - bottom_edge.cs_coord >= self.blue_shift { + } else if zone.cs_top_edge.wrapping_sub(bottom_edge.cs_coord) >= self.blue_shift { // Guarantee minimum of 1 pixel overshoot bottom_edge .ds_coord @@ -303,7 +303,7 @@ impl HintState { // Top edge captured by top zone. adjustment = if self.suppress_overshoot { zone.ds_flat_edge - } else if top_edge.cs_coord - zone.cs_bottom_edge >= self.blue_shift { + } else if top_edge.cs_coord.wrapping_sub(zone.cs_bottom_edge) >= self.blue_shift { // Guarantee minimum of 1 pixel overshoot top_edge .ds_coord