From fdfdf6ed9fed301f22c444303b8c9f7d80921bce Mon Sep 17 00:00:00 2001 From: mmmaarian03-crypto Date: Mon, 9 Nov 2026 04:14:27 +0200 Subject: [PATCH] Attachments: a saved attachment keeps the PDF's Mark-of-the-Web on Windows (#3990) (#4091) --- crates/platform/src/lib.rs | 1 + crates/platform/src/zone.rs | 150 +++++++++++++++++++++++++++++ crates/ui-egui/src/app/files.rs | 9 +- crates/ui-egui/tests/it/pickers.rs | 28 ++++++ parity/acrobat-features.toml | 4 +- 5 files changed, 189 insertions(+), 3 deletions(-) create mode 100644 crates/platform/src/zone.rs diff --git a/crates/platform/src/lib.rs b/crates/platform/src/lib.rs index 684f100d3a..ba8f8ab50a 100644 --- a/crates/platform/src/lib.rs +++ b/crates/platform/src/lib.rs @@ -5,3 +5,4 @@ pub mod desktop_theme; pub mod staging; +pub mod zone; diff --git a/crates/platform/src/zone.rs b/crates/platform/src/zone.rs new file mode 100644 index 0000000000..ba9bf305e9 --- /dev/null +++ b/crates/platform/src/zone.rs @@ -0,0 +1,150 @@ +//! Mark-of-the-Web (#3990): the `Zone.Identifier` stream Windows keeps beside a file that came +//! from the internet. SmartScreen, Office's Protected View and archive tools read it to decide how +//! far to trust the file. A file saved out of a document (an attachment) gets the zone of the +//! document it came from, so a PDF from an email doesn't turn the files inside it into trusted +//! local ones. + +use std::path::Path; + +/// What Windows writes for a file from the internet (`URLZONE_INTERNET`). A file saved out of a +/// document that carries no zone gets this one: where the document came from is unknown. +pub const INTERNET_ZONE: &str = "[ZoneTransfer]\r\nZoneId=3\r\n"; + +/// A zone stream is a few short lines; a larger one isn't copied. +#[cfg(windows)] +const MAX_ZONE_STREAM: u64 = 64 * 1024; + +/// Give `target` the zone of `source`: its `Zone.Identifier` stream, copied as it is, or the +/// Internet zone when `source` is `None` or has no zone. Windows only; elsewhere nothing is +/// written. Fails where the volume keeps no alternate data streams (FAT32, exFAT, some network +/// shares), so callers treat the mark as best effort. +pub fn mark_from(target: &Path, source: Option<&Path>) -> std::io::Result<()> { + #[cfg(windows)] + { + let zone = source.and_then(read_zone).unwrap_or_else(|| INTERNET_ZONE.as_bytes().to_vec()); + std::fs::write(zone_stream(target), zone) + } + #[cfg(not(windows))] + { + let _ = (target, source); + Ok(()) + } +} + +/// `path`'s `Zone.Identifier` stream, when it has one that reads as a zone. +#[cfg(windows)] +pub fn read_zone(path: &Path) -> Option> { + use std::io::Read; + let file = std::fs::File::open(zone_stream(path)).ok()?; + let mut zone = Vec::new(); + file.take(MAX_ZONE_STREAM.saturating_add(1)).read_to_end(&mut zone).ok()?; + let fits = u64::try_from(zone.len()).is_ok_and(|n| n <= MAX_ZONE_STREAM); + (fits && has_zone_id(&zone)).then_some(zone) +} + +/// Whether `stream` has a `ZoneId=` entry inside its `[ZoneTransfer]` section. Text that +/// only mentions those words elsewhere (say a `HostUrl` ending in `?ZoneId=3`) is not a zone. +#[cfg(windows)] +fn has_zone_id(stream: &[u8]) -> bool { + let mut in_section = false; + for line in stream.split(|&b| b == b'\n').map(<[u8]>::trim_ascii) { + if line.starts_with(b"[") { + in_section = line.eq_ignore_ascii_case(b"[ZoneTransfer]"); + continue; + } + let Some(eq) = line.iter().position(|&b| b == b'=') else { continue }; + let (key, value) = line.split_at(eq); + let value = value.get(1..).unwrap_or_default().trim_ascii(); + if in_section && key.trim_ascii().eq_ignore_ascii_case(b"ZoneId") && (1..=10).contains(&value.len()) && value.iter().all(u8::is_ascii_digit) { + return true; + } + } + false +} + +/// The path of `path`'s `Zone.Identifier` alternate data stream. +#[cfg(windows)] +fn zone_stream(path: &Path) -> std::path::PathBuf { + let mut stream = path.as_os_str().to_owned(); + stream.push(":Zone.Identifier"); + stream.into() +} + +#[cfg(test)] +#[cfg(windows)] +mod tests { + use super::*; + + /// A fresh folder in the temp directory, removed on drop. + struct Dir(std::path::PathBuf); + + impl Dir { + fn new(name: &str) -> Dir { + use std::sync::atomic::{AtomicU64, Ordering}; + static NEXT: AtomicU64 = AtomicU64::new(0); + let path = std::env::temp_dir().join(format!("pdfcraft-zone-{name}-{}-{}", std::process::id(), NEXT.fetch_add(1, Ordering::Relaxed))); + let _ = std::fs::remove_dir_all(&path); + std::fs::create_dir_all(&path).unwrap(); + Dir(path) + } + } + + impl Drop for Dir { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + #[test] + fn a_file_from_a_document_without_a_zone_gets_the_internet_zone() { + let dir = Dir::new("none"); + let target = dir.0.join("invoice.exe"); + std::fs::write(&target, b"MZ").unwrap(); + mark_from(&target, None).unwrap(); + assert_eq!(read_zone(&target).unwrap(), INTERNET_ZONE.as_bytes()); + let source = dir.0.join("local.pdf"); + std::fs::write(&source, b"%PDF").unwrap(); + mark_from(&target, Some(&source)).unwrap(); + assert_eq!(read_zone(&target).unwrap(), INTERNET_ZONE.as_bytes()); + assert_eq!(std::fs::read(&target).unwrap(), b"MZ", "the file's own bytes are untouched"); + } + + #[test] + fn a_file_from_a_document_gets_that_documents_zone() { + let dir = Dir::new("copy"); + let source = dir.0.join("from mail.pdf"); + std::fs::write(&source, b"%PDF").unwrap(); + let zone = "[ZoneTransfer]\r\nZoneId=3\r\nReferrerUrl=https://mail.example/\r\nHostUrl=https://mail.example/a.pdf\r\n"; + std::fs::write(zone_stream(&source), zone).unwrap(); + let target = dir.0.join("macro.docm"); + std::fs::write(&target, b"PK").unwrap(); + mark_from(&target, Some(&source)).unwrap(); + assert_eq!(read_zone(&target).unwrap(), zone.as_bytes()); + } + + #[test] + fn a_stream_that_isnt_a_zone_is_not_copied() { + let dir = Dir::new("junk"); + let source = dir.0.join("odd.pdf"); + std::fs::write(&source, b"%PDF").unwrap(); + std::fs::write(zone_stream(&source), b"not a zone").unwrap(); + let target = dir.0.join("notes.txt"); + std::fs::write(&target, b"hello").unwrap(); + mark_from(&target, Some(&source)).unwrap(); + assert_eq!(read_zone(&target).unwrap(), INTERNET_ZONE.as_bytes()); + for odd in [ + "[ZoneTransfer]\r\nHostUrl=https://example.test/?ZoneId=3\r\n", + "[Other]\r\nZoneId=3\r\n[ZoneTransfer]\r\nReferrerUrl=x\r\n", + "[ZoneTransfer]\r\nZoneId=\r\n", + "[ZoneTransfer]\r\nZoneId=three\r\n", + ] { + std::fs::write(zone_stream(&source), odd).unwrap(); + mark_from(&target, Some(&source)).unwrap(); + assert_eq!(read_zone(&target).unwrap(), INTERNET_ZONE.as_bytes(), "{odd:?}"); + } + let huge = [INTERNET_ZONE.as_bytes(), &vec![b'x'; 70 * 1024]].concat(); + std::fs::write(zone_stream(&source), huge).unwrap(); + mark_from(&target, Some(&source)).unwrap(); + assert_eq!(read_zone(&target).unwrap(), INTERNET_ZONE.as_bytes()); + } +} diff --git a/crates/ui-egui/src/app/files.rs b/crates/ui-egui/src/app/files.rs index c8ac133a00..ff7e06f377 100644 --- a/crates/ui-egui/src/app/files.rs +++ b/crates/ui-egui/src/app/files.rs @@ -442,6 +442,8 @@ impl PdfCraftApp { let Some(d) = self.session.get(doc) else { return }; let Some(att) = d.info.attachments.get(index).cloned() else { return }; let data = pdfcraft_render::attachment_data(&d.bytes, d.password.as_deref(), &att); + #[cfg(not(target_arch = "wasm32"))] + let source = d.path.clone(); match (data, open) { (Err(e), _) => self.notify_fmt("Couldn't read {name}: {e}", &[("name", &att.name), ("e", &e.to_string())]), (Ok(bytes), true) => { @@ -456,7 +458,12 @@ impl PdfCraftApp { pickers::Ask::Save(rfd::AsyncFileDialog::new().set_file_name(crate::files::attachment_file_name(&att.name))), None, move |app, path| match crate::editing::write_atomically(&path.to_string_lossy(), &bytes) { - Ok(()) => app.notify_fmt("Saved {name}", &[("name", &path.display().to_string())]), + Ok(()) => { + // Windows: the file keeps the document's Mark-of-the-Web (#3990). Best + // effort: a volume without alternate data streams can't carry it. + let _ = pdfcraft_platform::zone::mark_from(&path, source.as_deref().map(std::path::Path::new)); + app.notify_fmt("Saved {name}", &[("name", &path.display().to_string())]) + } Err(e) => app.notify_fmt("Couldn't save: {e}", &[("e", &e.to_string())]), }, ); diff --git a/crates/ui-egui/tests/it/pickers.rs b/crates/ui-egui/tests/it/pickers.rs index 330cd50667..0da954dd4c 100644 --- a/crates/ui-egui/tests/it/pickers.rs +++ b/crates/ui-egui/tests/it/pickers.rs @@ -1032,6 +1032,34 @@ fn attachment_save_preserves_bytes_metadata_and_selected_destination() { assert_eq!(doc.bytes.as_ref(), &original); } +/// Windows: a saved attachment keeps the Mark-of-the-Web of the PDF it came from, and gets the +/// Internet zone when that PDF has none or isn't a file (#3990). +#[cfg(windows)] +#[test] +fn a_saved_attachment_carries_the_documents_zone() { + let dir = AttachmentDir::new(); + let zone_of = |p: &std::path::Path| std::fs::read(format!("{}:Zone.Identifier", p.display())).ok(); + let mail = dir.0.join("from mail.pdf"); + std::fs::write(&mail, attachment_fixture()).unwrap(); + let zone = "[ZoneTransfer]\r\nZoneId=3\r\nHostUrl=https://mail.example/invoice.pdf\r\n"; + std::fs::write(format!("{}:Zone.Identifier", mail.display()), zone).unwrap(); + let local = dir.0.join("local.pdf"); + std::fs::write(&local, attachment_fixture()).unwrap(); + let mut h = harness(&[]); + let opened = [(Some(&mail), zone), (Some(&local), pdfcraft_platform::zone::INTERNET_ZONE), (None, pdfcraft_platform::zone::INTERNET_ZONE)]; + for (i, (path, expected)) in opened.into_iter().enumerate() { + h.state_mut().open_bytes(&format!("doc{i}.pdf"), path.map(|p| p.to_string_lossy().into_owned()), attachment_fixture()).unwrap(); + h.run_steps(2); + let id = h.state().views.last().unwrap().id; + let chosen = dir.0.join(format!("saved{i}.exe")); + h.state_mut().pick_override = Some(vec![chosen.to_string_lossy().into_owned()]); + h.state_mut().attachment_action(id, 0, false); + h.run_steps(2); + assert_eq!(std::fs::read(&chosen).unwrap(), b"hello notes"); + assert_eq!(zone_of(&chosen).as_deref(), Some(expected.as_bytes()), "document {i}"); + } +} + /// Every file under `dir`, recursively, sorted. fn files_under(dir: &std::path::Path) -> Vec { let mut out = Vec::new(); diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 9285f4bac9..d186bb2c7c 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -1322,8 +1322,8 @@ milestone = "M3" status = "shipped" commands = [] tools = ["doc_info"] -evidence = ["crates/render/src/inspect/tests.rs::attachments_of_both_kinds_are_listed_and_extracted", "crates/ui-egui/tests/it/attachment_row.rs::the_right_click_menu_opens_saves_and_deletes", "crates/ui-egui/tests/it/attachment_row.rs::double_clicking_a_pdf_attachment_opens_it_in_a_new_tab"] -notes = "Each row has Save, Open in a new tab (PDFs) and Delete (the document's own attachments) buttons, the same items on a right-click, and a double-click that opens a PDF in a new tab or saves any other file (#3466). Not yet: Acrobat's Search attachments, and opening a non-PDF attachment in another program (it is saved instead)." +evidence = ["crates/render/src/inspect/tests.rs::attachments_of_both_kinds_are_listed_and_extracted", "crates/ui-egui/tests/it/attachment_row.rs::the_right_click_menu_opens_saves_and_deletes", "crates/ui-egui/tests/it/attachment_row.rs::double_clicking_a_pdf_attachment_opens_it_in_a_new_tab", "crates/ui-egui/tests/it/pickers.rs::a_saved_attachment_carries_the_documents_zone"] +notes = "Each row has Save, Open in a new tab (PDFs) and Delete (the document's own attachments) buttons, the same items on a right-click, and a double-click that opens a PDF in a new tab or saves any other file (#3466). On Windows a saved attachment keeps the Mark-of-the-Web (Zone.Identifier) of the PDF it came from, or gets the Internet zone when the PDF has none (#3990). Not yet: Acrobat's Search attachments, and opening a non-PDF attachment in another program (it is saved instead)." [[feature]] id = "view.layers-panel"