Merge PR #348 from marcoarnulfo: Open: a password-protected presentation says so, and open errors show on the start screen

crates/engine/src/cmd/file.rs

@@ -82,6 +82,9 @@ pub fn open_presentation(name: &str, bytes: &[u8]) -> Result<Presentation> {
         deckcraft_format::repair(&mut p);
         return Ok(p);
     }
+    if deckcraft_pptx::is_encrypted(bytes) {
+        return Err(EngineError::Other(format!("{name} is password-protected. DeckCraft can't open password-protected presentations yet.")));
+    }
     let lower = name.to_ascii_lowercase();
     if lower.ends_with(".txt") || lower.ends_with(".md") {
         let text = String::from_utf8_lossy(bytes);

crates/pptx/src/cfb.rs

@@ -0,0 +1,185 @@
+//! Just enough of the Compound File Binary (CFB/OLE2) container to tell a password-protected
+//! presentation apart: PowerPoint wraps an encrypted `.pptx` in a CFB holding `EncryptionInfo`
+//! and `EncryptedPackage` streams (ECMA-376 / [MS-OFFCRYPTO]) instead of a zip. Every read is
+//! bounds-checked and every loop bounded. Copied from GridCraft's `crates/xlsx/src/cfb.rs`.
+
+/// The CFB signature at the start of the file.
+const SIGNATURE: [u8; 8] = [0xD0, 0xCF, 0x11, 0xE0, 0xA1, 0xB1, 0x1A, 0xE1];
+
+const END_OF_CHAIN: u32 = 0xFFFF_FFFE;
+/// Sector ids at or above this are markers (free, end of chain, FAT/DIFAT sector), not sectors.
+const MAX_REG_SECT: u32 = 0xFFFF_FFFA;
+/// Directory sectors followed at most (4096 entries with 512-byte sectors).
+const MAX_DIR_SECTORS: usize = 1024;
+/// DIFAT sectors followed at most.
+const MAX_DIFAT_SECTORS: usize = 1024;
+
+/// Stream names that mark an encrypted (password-protected) OOXML package.
+const ENCRYPTED_STREAMS: [&str; 2] = ["EncryptedPackage", "EncryptionInfo"];
+
+fn u16_at(b: &[u8], at: usize) -> Option<u16> {
+    let s = b.get(at..at.checked_add(2)?)?;
+    Some(u16::from_le_bytes([*s.first()?, *s.get(1)?]))
+}
+
+fn u32_at(b: &[u8], at: usize) -> Option<u32> {
+    let s = b.get(at..at.checked_add(4)?)?;
+    Some(u32::from_le_bytes([*s.first()?, *s.get(1)?, *s.get(2)?, *s.get(3)?]))
+}
+
+/// Whether `bytes` is a compound file holding an encryption stream. Walks the directory through
+/// the FAT; when the structure can't be followed (truncated or malformed), falls back to scanning
+/// the bytes for the UTF-16LE stream names.
+pub(crate) fn is_encrypted(bytes: &[u8]) -> bool {
+    if !bytes.starts_with(&SIGNATURE) {
+        return false;
+    }
+    match directory_names(bytes) {
+        Some(names) => names.iter().any(|n| ENCRYPTED_STREAMS.iter().any(|e| n.eq_ignore_ascii_case(e))),
+        None => ENCRYPTED_STREAMS.iter().any(|e| contains_utf16(bytes, e)),
+    }
+}
+
+fn contains_utf16(hay: &[u8], needle: &str) -> bool {
+    let n: Vec<u8> = needle.encode_utf16().flat_map(u16::to_le_bytes).collect();
+    !n.is_empty() && hay.windows(n.len()).any(|w| w == n.as_slice())
+}
+
+/// Names of every directory entry, or `None` when the directory can't be read.
+fn directory_names(b: &[u8]) -> Option<Vec<String>> {
+    if !b.starts_with(&SIGNATURE) {
+        return None;
+    }
+    let shift = u16_at(b, 30)?;
+    if shift != 9 && shift != 12 {
+        return None;
+    }
+    let size = 1usize << shift;
+    let per = size / 4;
+    let sector = |id: u32| -> Option<&[u8]> {
+        let start = (id as usize).checked_add(1)?.checked_mul(size)?;
+        b.get(start..start.checked_add(size)?)
+    };
+    // The DIFAT: the FAT's sector ids, 109 in the header then chained DIFAT sectors.
+    let mut difat: Vec<u32> = (0..109).filter_map(|i| u32_at(b, 76 + i * 4)).filter(|&s| s < MAX_REG_SECT).collect();
+    let mut next = u32_at(b, 68)?;
+    for _ in 0..MAX_DIFAT_SECTORS {
+        if next >= MAX_REG_SECT {
+            break;
+        }
+        let s = sector(next)?;
+        difat.extend((0..per - 1).filter_map(|i| u32_at(s, i * 4)).filter(|&x| x < MAX_REG_SECT));
+        next = u32_at(s, (per - 1) * 4)?;
+    }
+    let fat_next = |id: u32| -> Option<u32> {
+        let idx = id as usize;
+        let fat_sector = *difat.get(idx / per)?;
+        u32_at(sector(fat_sector)?, (idx % per) * 4)
+    };
+    let mut names = Vec::new();
+    let mut id = u32_at(b, 48)?;
+    for _ in 0..MAX_DIR_SECTORS {
+        if id == END_OF_CHAIN {
+            return Some(names);
+        }
+        if id >= MAX_REG_SECT {
+            return None;
+        }
+        let s = sector(id)?;
+        for e in s.as_chunks::<128>().0 {
+            let len = usize::from(u16_at(e, 64)?);
+            if !(2..=64).contains(&len) {
+                continue;
+            }
+            let units: Vec<u16> = (0..(len - 2) / 2).filter_map(|i| u16_at(e, i * 2)).collect();
+            names.push(String::from_utf16_lossy(&units));
+        }
+        id = fat_next(id)?;
+    }
+    // A directory longer than the bound (or a cyclic chain): report what was read.
+    Some(names)
+}
+
+#[cfg(test)]
+mod tests {
+    #![allow(clippy::unwrap_used)]
+
+    use super::*;
+
+    /// A minimal valid compound file (512-byte sectors: FAT at sector 0, directory at sector 1)
+    /// whose directory holds a root entry plus the given stream names.
+    fn build(streams: &[&str]) -> Vec<u8> {
+        let mut b = vec![0u8; 512 * 3];
+        b[..8].copy_from_slice(&SIGNATURE);
+        b[24..26].copy_from_slice(&0x3Eu16.to_le_bytes()); // minor version
+        b[26..28].copy_from_slice(&3u16.to_le_bytes()); // major version
+        b[28..30].copy_from_slice(&0xFFFEu16.to_le_bytes()); // byte order
+        b[30..32].copy_from_slice(&9u16.to_le_bytes()); // sector shift
+        b[32..34].copy_from_slice(&6u16.to_le_bytes()); // mini sector shift
+        b[44..48].copy_from_slice(&1u32.to_le_bytes()); // FAT sectors
+        b[48..52].copy_from_slice(&1u32.to_le_bytes()); // first directory sector
+        b[60..64].copy_from_slice(&END_OF_CHAIN.to_le_bytes()); // no mini FAT
+        b[68..72].copy_from_slice(&END_OF_CHAIN.to_le_bytes()); // no DIFAT sectors
+        for i in 0..109 {
+            b[76 + i * 4..80 + i * 4].copy_from_slice(&0xFFFF_FFFFu32.to_le_bytes());
+        }
+        b[76..80].copy_from_slice(&0u32.to_le_bytes()); // FAT lives in sector 0
+        // FAT (sector 0, at byte 512): sector 0 is a FAT sector, sector 1 ends its chain.
+        for i in 0..128 {
+            b[512 + i * 4..516 + i * 4].copy_from_slice(&0xFFFF_FFFFu32.to_le_bytes());
+        }
+        b[512..516].copy_from_slice(&0xFFFF_FFFDu32.to_le_bytes());
+        b[516..520].copy_from_slice(&END_OF_CHAIN.to_le_bytes());
+        // Directory (sector 1, at byte 1024).
+        for (k, name) in std::iter::once("Root Entry").chain(streams.iter().copied()).take(4).enumerate() {
+            let e = 1024 + k * 128;
+            let units: Vec<u16> = name.encode_utf16().take(31).collect();
+            for (i, u) in units.iter().enumerate() {
+                b[e + i * 2..e + i * 2 + 2].copy_from_slice(&u.to_le_bytes());
+            }
+            b[e + 64..e + 66].copy_from_slice(&((units.len() as u16 + 1) * 2).to_le_bytes());
+            b[e + 66] = if k == 0 { 5 } else { 2 };
+        }
+        b
+    }
+
+    #[test]
+    fn reads_directory_names() {
+        let b = build(&["EncryptionInfo", "EncryptedPackage"]);
+        assert_eq!(directory_names(&b), Some(vec!["Root Entry".into(), "EncryptionInfo".into(), "EncryptedPackage".into()]));
+        assert!(is_encrypted(&b));
+        // A PowerPoint 97-2003 .ppt is a compound file too, without the encryption streams.
+        assert!(!is_encrypted(&build(&["PowerPoint Document", "Current User"])));
+        assert!(!is_encrypted(b"PK\x03\x04"));
+    }
+
+    #[test]
+    fn hostile_headers_never_panic() {
+        let mut b = build(&["PowerPoint Document"]);
+        for at in [30usize, 48, 68, 76, 512, 516, 1024 + 64] {
+            for v in [0u32, 1, 7, 0xFFFF_FFFA, 0xFFFF_FFFF, 0x7FFF_FFFF] {
+                let mut c = b.clone();
+                c[at..at + 4].copy_from_slice(&v.to_le_bytes());
+                let _ = is_encrypted(&c);
+                for cut in [0usize, 8, 100, 600, 1100] {
+                    let _ = is_encrypted(&c[..cut.min(c.len())]);
+                }
+            }
+        }
+        // A directory chain that loops back on itself stops at the bound.
+        b[516..520].copy_from_slice(&1u32.to_le_bytes());
+        assert!(!is_encrypted(&b));
+    }
+
+    /// #346: opening a password-protected presentation says so instead of failing as an unknown
+    /// file.
+    #[test]
+    fn a_password_protected_presentation_is_named_as_such() {
+        let locked = build(&["EncryptionInfo", "EncryptedPackage"]);
+        let err = deckcraft_engine::cmd::file::open_presentation("Locked.pptx", &locked).unwrap_err().to_string();
+        assert!(err.contains("Locked.pptx is password-protected"), "{err}");
+        let ppt = build(&["PowerPoint Document", "Current User"]);
+        let err = deckcraft_engine::cmd::file::open_presentation("Old.ppt", &ppt).unwrap_err().to_string();
+        assert!(!err.contains("password"), "{err}");
+    }
+}

crates/pptx/src/lib.rs

@@ -7,6 +7,7 @@
 #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
 #![forbid(unsafe_code)]
 
+mod cfb;
 mod custgeom;
 mod maps;
 mod opc;
@@ -47,6 +48,11 @@ pub fn sniff(bytes: &[u8]) -> bool {
     zip::ZipArchive::new(std::io::Cursor::new(bytes)).map(|z| z.index_for_name("[Content_Types].xml").is_some()).unwrap_or(false)
 }
 
+/// Is this a password-protected presentation (an encrypted package inside a compound file)?
+pub fn is_encrypted(bytes: &[u8]) -> bool {
+    cfb::is_encrypted(bytes)
+}
+
 /// Read a `.pptx` / `.potx` / `.ppsx` package.
 pub fn import(bytes: &[u8]) -> Result<Presentation, PptxError> {
     read::import(bytes)

crates/ui-egui/src/backstage.rs

@@ -573,7 +573,7 @@ fn recent_list(app: &mut SlideApp, ui: &mut Ui, query: &str, limit: usize) {
         if resp.on_hover_text(&path).clicked()
             && let Err(e) = app.open_path(&path)
         {
-            app.set_status(e);
+            app.notify(e);
         }
     }
 }

crates/ui-egui/src/backstage_tests.rs

@@ -406,6 +406,25 @@ fn the_start_screen_is_file_home() {
     assert!(h.state().backstage.is_none());
 }
 
+/// #346: a file picked in Open that can't be read used to drop back to the start screen without
+/// a word. The reason now shows over the start screen.
+#[test]
+fn a_file_that_cant_be_opened_says_why() {
+    let dir = temp_dir("unreadable");
+    let path = dir.join("Notes.pptx");
+    std::fs::write(&path, b"not a presentation").unwrap();
+    let picked = path.to_string_lossy().to_string();
+    let services = Services { pick_open: Some(Box::new(move |_: &str| Some(picked.clone()))), ..Default::default() };
+    let mut h = harness(SlideApp::new(Session::new(), services));
+    h.state_mut().run("app.backstage", json!({"page": "open"})).unwrap();
+    h.run_steps(2);
+    h.get_by_label("Browse").click();
+    h.run_steps(3);
+    assert!(h.state().session.documents().is_empty());
+    assert!(h.query_by_label_contains("not a presentation DeckCraft can read").is_some(), "the error is on screen");
+    let _ = std::fs::remove_dir_all(&dir);
+}
+
 /// Print previews the pages its settings make and prints through a PDF opened in the system
 /// viewer.
 #[test]

crates/ui-egui/src/lib.rs

@@ -297,6 +297,12 @@ impl SlideApp {
         self.status = Some((msg.into(), now_ms()));
     }
 
+    /// A message that must be seen wherever the user is: a toast, which also shows over the start
+    /// screen and the File tab, where there is no status bar.
+    pub fn notify(&mut self, msg: impl Into<String>) {
+        self.toast = Some(toast::Toast { text: msg.into(), at: now_ms() });
+    }
+
     /// Run a command by id (UI commands here, everything else in the engine). Errors show in the
     /// status bar and are returned.
     pub fn run(&mut self, id: &str, params: Value) -> Result<Value, String> {
@@ -589,8 +595,10 @@ impl SlideApp {
 
     fn pick_and_open(&mut self) {
         if let Some(pick) = self.services.pick_open.as_mut() {
-            if let Some(path) = pick("open") {
-                let _ = self.open_path(&path);
+            if let Some(path) = pick("open")
+                && let Err(e) = self.open_path(&path)
+            {
+                self.notify(e);
             }
         } else if let Some(f) = self.services.open_async.as_mut() {
             f("open");
@@ -738,8 +746,10 @@ impl SlideApp {
         if self.session.active().is_none() && cmd != "file.openBytes" {
             let _ = self.session.execute("file.new", &json!({}));
         }
-        if let Err(e) = self.session.execute(cmd, &p) {
-            self.set_status(e.to_string());
+        match self.session.execute(cmd, &p) {
+            Err(e) if cmd == "file.openBytes" => self.notify(e.to_string()),
+            Err(e) => self.set_status(e.to_string()),
+            Ok(_) => {}
         }
     }