diff --git a/crates/odf/README.md b/crates/odf/README.md index e91d0d94..7e82a42b 100644 --- a/crates/odf/README.md +++ b/crates/odf/README.md @@ -7,7 +7,7 @@ this crate reads and writes that container and nothing inside it. | Module | What it does | | --- | --- | | `Package`, `PackageWriter` | The ZIP container: the `mimetype` file, the manifest, the files, within `Limits`. | -| `Manifest` | `META-INF/manifest.xml`: each file's media type, and how an encrypted file is encrypted. Read by namespace, whatever prefix the file uses. | +| `Manifest` | `META-INF/manifest.xml`: each file's media type, and how an encrypted file is encrypted. Read by namespace, whatever prefix the file uses; `parse_strict` / `Package::manifest_strict` refuse a damaged one instead of reading what they can. | | `crypt` | Password-protected packages: AES-256-CBC, SHA-256 start key, PBKDF2 (HMAC-SHA-1), SHA-256 or SHA-1 checksums. A package in, a package out. | | `flat` | Flat single-file documents (`.fodt`, `.fods`, `.fodp`, `.fodg`): one `office:document` taken apart into a package's files, and a package put together into one. | | `zip_guard` | Refuses a ZIP whose end record declares too many entries, before `zip` reserves room for them. | diff --git a/crates/odf/src/lib.rs b/crates/odf/src/lib.rs index 174eb50c..ff41b351 100644 --- a/crates/odf/src/lib.rs +++ b/crates/odf/src/lib.rs @@ -76,6 +76,10 @@ pub enum Error { /// A flat document that is no `office:document`, or not UTF-8. #[error("not a flat OpenDocument file ({0})")] Flat(&'static str), + /// A manifest that [`Manifest::parse_strict`] refuses: malformed XML, a document type + /// declaration, a root that is no `manifest:manifest`, or more entries than it reads. + #[error("the package's manifest is not valid ({0})")] + Manifest(String), } impl Error { diff --git a/crates/odf/src/manifest.rs b/crates/odf/src/manifest.rs index d355c604..89d4f063 100644 --- a/crates/odf/src/manifest.rs +++ b/crates/odf/src/manifest.rs @@ -5,7 +5,9 @@ use std::fmt::Write as _; use base64::Engine as _; use base64::engine::general_purpose::STANDARD; -use craft_xml::{Canonical, El, Options, esc_attr}; +use craft_xml::{Canonical, Doctype, El, Options, esc_attr}; + +use crate::Error; /// The manifest's path in a package. pub const MANIFEST_PATH: &str = "META-INF/manifest.xml"; @@ -76,6 +78,29 @@ impl Manifest { Manifest { version: attr(&root, "version").map(str::to_string), entries } } + /// Reads a manifest, refusing one a lenient read would repair: malformed or unbalanced + /// XML, a document type declaration (of any kind), an undefined entity, text or a second + /// element outside the root, a root that is not `manifest` in the manifest namespace, a + /// `file-entry` without a `full-path`, or more than [`MAX_ENTRIES`] entries. For a reader + /// that refuses a damaged package rather than open what it can. + pub fn parse_strict(xml: &[u8]) -> Result { + let bad = |why: String| Error::Manifest(why); + let canonical = Canonical::new(&[("manifest", MANIFEST_NS)]); + let opts = Options { doctype: Doctype::Reject, ..Options::canonical(&canonical).strict() }; + let root = opts.parse(xml).map_err(|e| bad(e.to_string()))?; + if root.name != "manifest:manifest" { + return Err(bad(format!("the root element is {}, not manifest:manifest", root.name))); + } + let mut entries = Vec::new(); + for e in root.elements().filter(|e| e.name == "manifest:file-entry") { + if entries.len() == MAX_ENTRIES { + return Err(bad(format!("more than {MAX_ENTRIES} file entries"))); + } + entries.push(file_entry(e).ok_or_else(|| bad("a file entry has no full-path".into()))?); + } + Ok(Manifest { version: attr(&root, "version").map(str::to_string), entries }) + } + /// The entry for `path`. pub fn entry(&self, path: &str) -> Option<&FileEntry> { self.entries.iter().find(|e| e.path == path) diff --git a/crates/odf/src/package.rs b/crates/odf/src/package.rs index 89977223..48272e85 100644 --- a/crates/odf/src/package.rs +++ b/crates/odf/src/package.rs @@ -135,6 +135,13 @@ impl<'a> Package<'a> { self.read(MANIFEST_PATH).ok().flatten().map(|xml| Manifest::parse(&xml)).unwrap_or_default() } + /// The package's manifest read with [`Manifest::parse_strict`]: an error when the package + /// has none, or one a strict read refuses, or it is over a limit. + pub fn manifest_strict(&mut self) -> Result { + let xml = self.read(MANIFEST_PATH)?.ok_or_else(|| Error::Manifest("the package has no manifest".into()))?; + Manifest::parse_strict(&xml) + } + /// Does the manifest say any file is encrypted ([`crate::crypt`])? pub fn is_encrypted(&mut self) -> bool { self.manifest().is_encrypted() diff --git a/crates/odf/src/tests.rs b/crates/odf/src/tests.rs index 0894271a..7a7d2eac 100644 --- a/crates/odf/src/tests.rs +++ b/crates/odf/src/tests.rs @@ -68,6 +68,48 @@ fn a_manifest_is_read_by_namespace_and_survives_damage() { assert_eq!(Manifest::parse(many.as_bytes()).entries.len(), crate::manifest::MAX_ENTRIES); } +#[test] +fn a_strict_manifest_refuses_what_a_lenient_one_repairs() { + const NS: &str = r#"xmlns:manifest="urn:oasis:names:tc:opendocument:xmlns:manifest:1.0""#; + let good = format!( + r#""# + ); + let m = Manifest::parse_strict(good.as_bytes()).unwrap(); + assert_eq!((m.version.as_deref(), m.media_type("/")), (Some("1.3"), Some("a/b"))); + // Another prefix for the same namespace is the same manifest. + let other = r#""#; + assert_eq!(Manifest::parse_strict(other.as_bytes()), Ok(m.clone())); + for bad in [ + format!(r#""#), + format!(r#""#), + format!(r#"]>"#), + format!(r#""#), + format!(r#"trailing"#), + format!(r#""#), + format!(r#""#), + format!(r#""#), + r#""#.to_string(), + r#""#.to_string(), + String::new(), + ] { + assert!(matches!(Manifest::parse_strict(bad.as_bytes()), Err(Error::Manifest(_))), "{bad}"); + let _ = Manifest::parse(bad.as_bytes()); + } + // The lenient read takes what it can of the first. + assert_eq!(Manifest::parse(format!(r#""#).as_bytes()).entries.len(), 1); + // Entries: up to the cap, not past it. + let many = |n: usize| format!(r#"{}"#, r#""#.repeat(n)); + assert_eq!(Manifest::parse_strict(many(crate::manifest::MAX_ENTRIES).as_bytes()).map(|m| m.entries.len()), Ok(crate::manifest::MAX_ENTRIES)); + assert!(Manifest::parse_strict(many(crate::manifest::MAX_ENTRIES + 1).as_bytes()).is_err()); + // Through a package: missing and present. + let mut p_bytes = zip_of(&[("mimetype", mime::SPREADSHEET.as_bytes())]); + assert!(matches!(Package::open(&p_bytes, &Limits::DEFAULT).unwrap().manifest_strict(), Err(Error::Manifest(_)))); + p_bytes = zip_of(&[("mimetype", mime::SPREADSHEET.as_bytes()), ("META-INF/manifest.xml", good.as_bytes())]); + assert_eq!(Package::open(&p_bytes, &Limits::DEFAULT).unwrap().manifest_strict(), Ok(m)); + // A written package passes. + assert!(Package::open(&secret(), &Limits::DEFAULT).unwrap().manifest_strict().is_ok_and(|m| m.entries.len() == 4)); +} + #[test] fn a_package_is_held_to_its_limits() { let bytes = zip_of(&[("mimetype", mime::SPREADSHEET.as_bytes()), ("a.xml", &[b'a'; 100]), ("b.xml", &[b'b'; 100])]);