Merge pull request #1657 from avhn/fix/1125-font-offset-overflow

M8: a font's damaged table offsets are skipped instead of wrapping on 32-bit targets (#1125)

crates/text/src/fontdb.rs

@@ -481,11 +481,12 @@ fn name_string(f: &skrifa::FontRef, id: StringId) -> Option<String> {
 }
 
 /// The normal (0) horizontal track of a `trak` table: (size, tracking in font units) by size. Empty
-/// when the table is missing, damaged or has no normal track.
+/// when the table is missing, damaged or has no normal track. A short size or value array invalidates
+/// the entire track.
 fn normal_track(t: &[u8]) -> Vec<(f64, f64)> {
-    let u16_at = |o: usize| t.get(o..o + 2).map(|b| u16::from_be_bytes([b[0], b[1]]));
-    let i16_at = |o: usize| t.get(o..o + 2).map(|b| i16::from_be_bytes([b[0], b[1]]));
-    let fixed_at = |o: usize| t.get(o..o + 4).map(|b| f64::from(i32::from_be_bytes([b[0], b[1], b[2], b[3]])) / 65536.0);
+    let u16_at = |o: usize| t.get(o..o.checked_add(2)?).map(|b| u16::from_be_bytes([b[0], b[1]]));
+    let i16_at = |o: usize| t.get(o..o.checked_add(2)?).map(|b| i16::from_be_bytes([b[0], b[1]]));
+    let fixed_at = |o: usize| t.get(o..o.checked_add(4)?).map(|b| f64::from(i32::from_be_bytes([b[0], b[1], b[2], b[3]])) / 65536.0);
     let Some(horiz) = u16_at(6).map(usize::from).filter(|o| *o > 0) else { return Vec::new() };
     let (Some(tracks), Some(sizes)) = (u16_at(horiz).map(usize::from), u16_at(horiz + 2).map(usize::from)) else { return Vec::new() };
     let Some(size_table) = t.get(horiz + 4..horiz + 8).map(|b| u32::from_be_bytes([b[0], b[1], b[2], b[3]]) as usize) else { return Vec::new() };
@@ -497,10 +498,10 @@ fn normal_track(t: &[u8]) -> Vec<(f64, f64)> {
         }
         let Some(values) = u16_at(entry + 6).map(usize::from) else { return Vec::new() };
         let out: Vec<(f64, f64)> = (0..sizes)
-            .filter_map(|i| Some((fixed_at(size_table + i * 4)?, f64::from(i16_at(values + i * 2)?))))
+            .filter_map(|i| Some((fixed_at(size_table.checked_add(i.checked_mul(4)?)?)?, f64::from(i16_at(values.checked_add(i * 2)?)?))))
             .filter(|(s, _)| s.is_finite())
             .collect();
-        return if out.windows(2).all(|w| w[0].0 < w[1].0) { out } else { Vec::new() };
+        return if out.len() == sizes && out.windows(2).all(|w| w[0].0 < w[1].0) { out } else { Vec::new() };
     }
     Vec::new()
 }
@@ -1321,14 +1322,17 @@ fn index_bytes(path: &std::path::Path, len: usize) -> Option<Vec<u8>> {
     } else {
         vec![0]
     };
-    for at in fonts {
-        let Some(tables) = fill(&mut buf, at, 12).and_then(|()| be(&buf, at + 4, 2)) else { continue };
-        if fill(&mut buf, at + 12, 16 * tables).is_none() {
+    'faces: for at in fonts {
+        let Some(tables) = fill(&mut buf, at, 12).and_then(|()| be(&buf, at.checked_add(4)?, 2)) else { continue };
+        let Some(dir) = at.checked_add(12) else { continue };
+        if fill(&mut buf, dir, 16 * tables).is_none() {
             continue;
         }
-        for rec in (0..tables).map(|t| at + 12 + 16 * t) {
-            if buf.get(rec..rec + 4) == Some(b"name")
-                && let (Some(off), Some(n)) = (be(&buf, rec + 8, 4), be(&buf, rec + 12, 4))
+        for t in 0..tables {
+            let Some(rec) = dir.checked_add(16 * t) else { continue 'faces };
+            let (Some(tag_end), Some(off_at), Some(len_at)) = (rec.checked_add(4), rec.checked_add(8), rec.checked_add(12)) else { continue 'faces };
+            if buf.get(rec..tag_end) == Some(b"name")
+                && let (Some(off), Some(n)) = (be(&buf, off_at, 4), be(&buf, len_at, 4))
             {
                 let _ = fill(&mut buf, off, n);
             }

crates/text/src/fontdb_tests.rs

@@ -73,6 +73,62 @@ fn the_scan_reads_names_like_a_whole_file_read() {
     }
 }
 
+fn indexed_test_font(bytes: &[u8], case: &str) -> Vec<u8> {
+    struct TempFile(std::path::PathBuf);
+    impl Drop for TempFile {
+        fn drop(&mut self) {
+            let _ = std::fs::remove_file(&self.0);
+        }
+    }
+    let file = TempFile(std::env::temp_dir().join(format!("figcraft-font-index-{case}-{}.ttc", std::process::id())));
+    std::fs::write(&file.0, bytes).unwrap();
+    index_bytes(&file.0, bytes.len()).unwrap()
+}
+
+fn synthetic_font_collection(first: u32) -> Vec<u8> {
+    let mut bytes = vec![0; 112];
+    bytes[..4].copy_from_slice(b"ttcf");
+    bytes[4..8].copy_from_slice(&0x0001_0000u32.to_be_bytes());
+    bytes[8..12].copy_from_slice(&2u32.to_be_bytes());
+    bytes[12..16].copy_from_slice(&first.to_be_bytes());
+    bytes[16..20].copy_from_slice(&32u32.to_be_bytes());
+    // The second face has a name table and an outline table. Only names should be read.
+    bytes[32..36].copy_from_slice(&0x0001_0000u32.to_be_bytes());
+    bytes[36..38].copy_from_slice(&2u16.to_be_bytes());
+    bytes[44..48].copy_from_slice(b"name");
+    bytes[52..56].copy_from_slice(&80u32.to_be_bytes());
+    bytes[56..60].copy_from_slice(&8u32.to_be_bytes());
+    bytes[60..64].copy_from_slice(b"glyf");
+    bytes[68..72].copy_from_slice(&96u32.to_be_bytes());
+    bytes[72..76].copy_from_slice(&4u32.to_be_bytes());
+    bytes[80..88].copy_from_slice(b"fontname");
+    bytes[96..100].copy_from_slice(b"skip");
+    bytes
+}
+
+#[test]
+fn index_bytes_skips_collection_offsets_near_u32_max() {
+    for offset in [u32::MAX - 11, u32::MAX - 1, u32::MAX] {
+        let bytes = synthetic_font_collection(offset);
+        let indexed = indexed_test_font(&bytes, "offset");
+        assert_eq!(&indexed[32..76], &bytes[32..76]);
+        assert_eq!(&indexed[80..88], b"fontname");
+        assert_eq!(&indexed[96..100], &[0; 4], "outline bytes stay unread");
+    }
+}
+
+#[test]
+fn index_bytes_skips_a_truncated_directory_and_reads_the_next_face() {
+    let mut bytes = synthetic_font_collection(100);
+    bytes[100..104].copy_from_slice(&0x0001_0000u32.to_be_bytes());
+    bytes[104..106].copy_from_slice(&2u16.to_be_bytes());
+    let indexed = indexed_test_font(&bytes, "directory");
+    assert_eq!(&indexed[100..112], &bytes[100..112], "the header fits but its directory does not");
+    assert_eq!(&indexed[32..76], &bytes[32..76]);
+    assert_eq!(&indexed[80..88], b"fontname");
+    assert_eq!(&indexed[96..100], &[0; 4], "outline bytes stay unread");
+}
+
 #[test]
 fn macos_font_assets_are_found_in_the_asset_store() {
     // A store laid out like macOS's: fonts downloaded on demand sit in `<id>.asset/AssetData`
@@ -403,6 +459,51 @@ fn named_styles_keep_their_optical_size() {
     assert_eq!(face.axis_values(&style)[opsz], face.axes[opsz].max);
 }
 
+fn synthetic_trak() -> Vec<u8> {
+    let mut bytes = vec![0; 46];
+    bytes[..4].copy_from_slice(&0x0001_0000u32.to_be_bytes());
+    bytes[6..8].copy_from_slice(&12u16.to_be_bytes());
+    bytes[12..14].copy_from_slice(&1u16.to_be_bytes());
+    bytes[14..16].copy_from_slice(&3u16.to_be_bytes());
+    bytes[16..20].copy_from_slice(&28u32.to_be_bytes());
+    // The normal track (0.0), with three sizes and their signed tracking values.
+    bytes[24..26].copy_from_slice(&256u16.to_be_bytes());
+    bytes[26..28].copy_from_slice(&40u16.to_be_bytes());
+    for (i, size) in [10i32, 20, 40].into_iter().enumerate() {
+        bytes[28 + i * 4..32 + i * 4].copy_from_slice(&(size * 65536).to_be_bytes());
+    }
+    for (i, value) in [-8i16, 0, 16].into_iter().enumerate() {
+        bytes[40 + i * 2..42 + i * 2].copy_from_slice(&value.to_be_bytes());
+    }
+    bytes
+}
+
+#[test]
+fn normal_track_reads_a_well_formed_table() {
+    assert_eq!(normal_track(&synthetic_trak()), [(10.0, -8.0), (20.0, 0.0), (40.0, 16.0)]);
+}
+
+#[test]
+fn normal_track_ignores_size_table_offsets_near_u32_max() {
+    let mut bytes = synthetic_trak();
+    // On 32-bit targets even the first size's end can overflow. On 64-bit these are out of bounds.
+    for offset in [u32::MAX - 7, u32::MAX - 3, u32::MAX - 1, u32::MAX] {
+        bytes[16..20].copy_from_slice(&offset.to_be_bytes());
+        assert!(normal_track(&bytes).is_empty(), "size table at {offset}");
+    }
+}
+
+#[test]
+fn normal_track_never_returns_a_partial_track_from_a_damaged_table() {
+    let mut bytes = synthetic_trak();
+    bytes.truncate(44); // Two values fit, but the third is missing.
+    assert!(normal_track(&bytes).is_empty());
+    let mut bytes = synthetic_trak();
+    bytes[26..28].copy_from_slice(&20u16.to_be_bytes());
+    bytes.truncate(36); // All values fit, but only two sizes do.
+    assert!(normal_track(&bytes).is_empty());
+}
+
 /// The tracking a font's `trak` table gives at a size (Apple's fonts carry one; the platform text
 /// engine applies it on its own), in pixels, interpolated between the sizes the table lists.
 #[test]