File formats: a PCX palette cut short is refused

An 8-bit PCX file whose image data is followed by the palette marker
(0x0C) and fewer than 768 color bytes was read as gray levels. The reader
now notes where the run-length encoded image data ends, and when the
palette marker follows it without all 768 colors, refuses the file as cut
short. An 8-bit file with nothing after its image data still reads as
gray levels, and a gray level of 12 that falls 769 bytes before the end of
such a file is read as a pixel instead of as the palette marker.

crates/engine/src/cmd/fileio/pcxread.rs

@@ -75,9 +75,24 @@ pub(super) fn decode(bytes: &[u8], max_alloc: u64) -> Result<image::RgbaImage, S
         return Err(format!("an image of {w} × {h} pixels is too large to draw"));
     }
     // An 8-bit image's 256-color palette is its last 769 bytes, `0x0C` and then the colors; the
-    // image data ends before it.
-    let tail = bytes.len().checked_sub(769).filter(|at| *at >= HEADER && (bits, planes) == (8, 1) && bytes.get(*at) == Some(&0x0c));
-    let data = unpack(bytes.get(HEADER..tail.unwrap_or(bytes.len())).unwrap_or_default(), scan * h)?;
+    // image data ends before it. A `0x0C` there is image data instead when the image data runs to
+    // the end of the file.
+    let candidate = bytes.len().checked_sub(769).filter(|at| *at >= HEADER && (bits, planes) == (8, 1) && bytes.get(*at) == Some(&0x0c));
+    let body = bytes.get(HEADER..).unwrap_or_default();
+    let (tail, decoded) = match candidate {
+        Some(at) => match unpack(body, scan * h) {
+            Ok((data, used)) if used == body.len() => (None, Ok((data, used))),
+            _ => (Some(at), unpack(bytes.get(HEADER..at).unwrap_or_default(), scan * h)),
+        },
+        None => (None, unpack(body, scan * h)),
+    };
+    let (data, used) = decoded?;
+    // A palette marker right after the image data with fewer than 768 colors after it is a palette
+    // cut short.
+    let after = bytes.get(HEADER + used..).unwrap_or_default();
+    if tail.is_none() && (bits, planes) == (8, 1) && after.first() == Some(&0x0c) && after.len() < 769 {
+        return Err(TRUNCATED.into());
+    }
     let palette: Vec<[u8; 3]> = match (bits, planes) {
         (8, 1) => match tail.and_then(|at| bytes.get(at + 1..)) {
             Some(colors) => colors.as_chunks::<3>().0.to_vec(),
@@ -133,8 +148,8 @@ fn cga(background: u8, settings: u8) -> Vec<[u8; 3]> {
 
 /// `len` bytes of run-length encoded image data: a byte with its top two bits set repeats the
 /// next byte as many times as its other six bits count, and any other byte stands for itself.
-/// Runs may cross the end of a scan line.
-fn unpack(data: &[u8], len: usize) -> Result<Vec<u8>, String> {
+/// Runs may cross the end of a scan line. Also returns how many bytes of `data` were read.
+fn unpack(data: &[u8], len: usize) -> Result<(Vec<u8>, usize), String> {
     let mut out = Vec::with_capacity(len);
     let mut bytes = data.iter();
     while out.len() < len {
@@ -147,5 +162,5 @@ fn unpack(data: &[u8], len: usize) -> Result<Vec<u8>, String> {
             out.push(b);
         }
     }
-    Ok(out)
+    Ok((out, data.len() - bytes.len()))
 }

crates/engine/src/cmd/fileio/tests_pcxread.rs

@@ -119,6 +119,13 @@ fn every_depth_and_plane_layout_reads() {
     // Without it, the index is a gray level.
     let f = pcx(5, 8, 1, 0, (2, 2), 2, 0, &[], &rle(&[0, 1, 254, 255]));
     assert_eq!(pixels(&f), [[0, 0, 0, 255], [1, 1, 1, 255], [254, 254, 254, 255], [255; 4]]);
+    // A gray level 12 769 bytes before the end of image data that runs to the end of the file.
+    let mut levels = vec![1; 1000];
+    levels[231] = 0x0c;
+    let f = pcx(5, 8, 1, 0, (1000, 1), 1000, 0, &[], &levels);
+    assert_eq!(f.len() - 769, 128 + 231);
+    let read = pixels(&f);
+    assert_eq!((read.len(), read[231], read[232]), (1000, [12, 12, 12, 255], [1, 1, 1, 255]));
     // A run may cross the end of a scan line.
     let f = pcx(5, 8, 1, 0, (2, 2), 2, 0, &[], &[0xc4, 9]);
     assert_eq!(pixels(&f), [[9, 9, 9, 255]; 4]);
@@ -175,6 +182,12 @@ fn damaged_and_unsupported_files_are_refused_with_a_message() {
     short.push(0x0c);
     short.extend([7; 768]);
     assert_eq!(message(&short), "the PCX file is cut short", "three pixels and a palette");
+    // A palette marker after the image data with fewer than 768 colors after it.
+    let mut cut_palette = gray.clone();
+    cut_palette.push(0x0c);
+    cut_palette.extend([7; 767]);
+    assert_eq!(message(&cut_palette), "the PCX file is cut short", "a palette cut short");
+    assert_eq!(message(&[&gray[..], &[0x0c]].concat()), "the PCX file is cut short", "a marker alone");
     let mut s = Session::new();
     let e = s.execute("document.open", &json!({"name": "a.pcx", "dataBase64": vectorcraft_format::base64_encode(&gray[..131])}));
     assert!(e.expect_err("refused").to_string().contains("the PCX file is cut short"));