LR-IMP-FORMATS: a SEF trailer counts only with nonempty blocks of their entry's type, and an EOI is looked for before it (#3350)

crates/codecs/src/jpeg.rs

@@ -185,7 +185,8 @@ fn without_sef_trailer(bytes: &[u8]) -> std::borrow::Cow<'_, [u8]> {
     use std::borrow::Cow;
     // The trailer is checked first: it reads only the file's last bytes, not the scan data.
     let Some(image) = sef_image_end(bytes).and_then(|end| bytes.get(..end)) else { return Cow::Borrowed(bytes) };
-    if reaches_eoi(bytes) {
+    // the image before the trailer: an FF D9 inside the SEF data doesn't end the scan
+    if reaches_eoi(image) {
         return Cow::Borrowed(bytes);
     }
     let mut fixed = Vec::with_capacity(image.len().saturating_add(2));
@@ -199,8 +200,9 @@ fn without_sef_trailer(bytes: &[u8]) -> std::borrow::Cow<'_, [u8]> {
 /// Where the image ends in a file carrying a Samsung SEF trailer: the start of its earliest data
 /// block. The trailer is the file's last bytes: `SEFH`, a u32 LE version and entry count, 12-byte
 /// entries (u16 padding, u16 type, u32 LE offset of the block back from `SEFH`, u32 LE length),
-/// then the u32 LE length of that directory and `SEFT`. `None` unless every entry's block lies
-/// inside the file, before `SEFH`.
+/// then the u32 LE length of that directory and `SEFT`. Each block starts with the same padding
+/// and type as its entry. `None` unless every entry's block is nonempty, starts with its entry's
+/// type and lies inside the file, before `SEFH`.
 fn sef_image_end(b: &[u8]) -> Option<usize> {
     let le32 = |at: usize| b.get(at..at.checked_add(4)?)?.try_into().ok().map(|v| u32::from_le_bytes(v) as usize);
     let tail = b.len().checked_sub(8)?;
@@ -220,7 +222,9 @@ fn sef_image_end(b: &[u8]) -> Option<usize> {
     for i in 0..count {
         let entry = entries.checked_add(i.checked_mul(12)?)?;
         let block = dir.checked_sub(le32(entry.checked_add(4)?)?)?;
-        if block.checked_add(le32(entry.checked_add(8)?)?)? > dir {
+        let len = le32(entry.checked_add(8)?)?;
+        let kind = |at: usize| b.get(at.checked_add(2)?..at.checked_add(4)?);
+        if len < 4 || block.checked_add(len)? > dir || kind(block)? != kind(entry)? {
             return None;
         }
         start = start.min(block);

crates/codecs/tests/it/header.rs

@@ -164,19 +164,26 @@ fn truncation_details() {
 /// `SEFH` directory with one entry (type 0x08E0) whose offset counts back from `SEFH` by `back`,
 /// the directory's length and `SEFT`.
 fn with_sef_trailer(image: &[u8], back: u32, magic: &[u8; 4]) -> Vec<u8> {
+    // marker-like bytes: both decoders refuse them as a second frame header after the scan
+    sef_file(image, back, magic, 0x08E0, &[0xFF, 0xC0, 0, 0x11, 0xFF, 0xDA, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12], None)
+}
+
+/// [`with_sef_trailer`] with the entry's type, the block's data after its name, and the length
+/// the entry gives (`None`: the block's own).
+fn sef_file(image: &[u8], back: u32, magic: &[u8; 4], kind: u16, data: &[u8], len: Option<u32>) -> Vec<u8> {
     let mut b = image.to_vec();
     let mut block = vec![0, 0, 0xE0, 0x08];
     block.extend_from_slice(&18u32.to_le_bytes());
     block.extend_from_slice(b"Panorama_Shot_Info");
-    // marker-like bytes: both decoders refuse them as a second frame header after the scan
-    block.extend_from_slice(&[0xFF, 0xC0, 0, 0x11, 0xFF, 0xDA, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]);
+    block.extend_from_slice(data);
     b.extend_from_slice(&block);
     let mut dir = magic.to_vec();
     dir.extend_from_slice(&106u32.to_le_bytes());
     dir.extend_from_slice(&1u32.to_le_bytes());
-    dir.extend_from_slice(&[0, 0, 0xE0, 0x08]);
+    dir.extend_from_slice(&[0, 0]);
+    dir.extend_from_slice(&kind.to_le_bytes());
     dir.extend_from_slice(&back.to_le_bytes());
-    dir.extend_from_slice(&(block.len() as u32).to_le_bytes());
+    dir.extend_from_slice(&len.unwrap_or(block.len() as u32).to_le_bytes());
     let len = dir.len() as u32;
     b.extend_from_slice(&dir);
     b.extend_from_slice(&len.to_le_bytes());
@@ -201,12 +208,19 @@ fn samsung_sef_trailer_without_eoi() {
     assert_eq!(full.image.data, reference.image.data, "the same pixels as the file with its EOI");
     let small = decode(&pano, DecodeOptions::fit(32, 32)).unwrap();
     assert_eq!((small.source_width, small.source_height), (160, 96));
-    // still truncated: no trailer, offsets outside the file or past `SEFH`, no `SEFH`, a cut
-    // before the first scan
+    // an FF D9 inside the SEF data is not the image's end
+    let eoi_in_data = sef_file(scan, block, b"SEFH", 0x08E0, &[0xFF, 0xC0, 0, 0x11, 0xFF, 0xD9, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12], None);
+    assert_eq!(read_header(&eoi_in_data).unwrap().width, 160);
+    let small = decode(&eoi_in_data, DecodeOptions::fit(32, 32)).unwrap();
+    assert_eq!((small.source_width, small.source_height), (160, 96));
+    // still truncated: no trailer, offsets outside the file or past `SEFH`, no `SEFH`, an empty
+    // entry, an entry whose type isn't its block's, a cut before the first scan
     assert!(read_header(scan).is_err());
     assert!(read_header(&with_sef_trailer(scan, u32::MAX, b"SEFH")).is_err());
     assert!(read_header(&with_sef_trailer(scan, block - 1, b"SEFH")).is_err());
     assert!(read_header(&with_sef_trailer(scan, block, b"SEFX")).is_err());
+    assert!(read_header(&sef_file(scan, 0, b"SEFH", 0x08E0, &[], Some(0))).is_err());
+    assert!(read_header(&sef_file(scan, block, b"SEFH", 0x0A01, &[0; 18], None)).is_err());
     let cut_in_headers = with_sef_trailer(&jpeg[..40], block, b"SEFH");
     assert!(read_header(&cut_in_headers).is_err());
     assert!(decode(&cut_in_headers, DecodeOptions::fit(32, 32)).is_err());