Avoid various overflows in CFF (#1196)

Add targeted `wrapping_add` and `wrapping_sub` calls to avoid panics on arithmetic overflows for the specific case in #1193
googlefonts · Oct 21, 2024 · 9ac54bd · 9ac54bd
1 parent 3662eb3
commit 9ac54bd
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 7 deletions.
diff --git 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
+    /// <https://gitlab.freedesktop.org/freetype/freetype/-/blob/57617782464411201ce7bbc93b086c1b4d7d84a5/src/cff/cffparse.c#L1431>
+    ///
     /// See <https://learn.microsoft.com/en-us/typography/opentype/spec/cff2#table-6-operand-types>
     pub fn apply_delta_prefix_sum(&mut self) {
         if self.top > 1 {
@@ -212,7 +215,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 +227,12 @@ impl Stack {
                 } else {
                     Fixed::from_i32(*value)
                 };
+                // See <https://github.com/googlefonts/fontations/issues/1193>
+                // 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
@@ -267,19 +267,22 @@ impl HintState {
     ///
     /// See <https://gitlab.freedesktop.org/freetype/freetype/-/blob/80a507a6b8e3d2906ad2c8ba69329bd2fb2a85ef/src/psaux/psblues.c#L465>
     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 <https://github.com/googlefonts/fontations/issues/1193>
         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 {
                     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
@@ -294,13 +297,13 @@ 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 {
                     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