From 7d9adcb8f1db2a3a7fe10fc37735c299c3a58dae Mon Sep 17 00:00:00 2001 From: Earl Co Date: Mon, 19 Oct 2026 08:48:40 +0200 Subject: [PATCH] fix(preflight): identify PDF standards by XML namespace (#3364) * fix(preflight): identify standards by XML namespace * fix(preflight): bound XMP attribute parsing work * fix(preflight): cap XMP during stream decoding * Preflight: read the PDF/A and PDF/UA identification by its usual prefix when the XMP can't be parsed as XML Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Brandon Thomas Co-authored-by: Claude Opus 5.5 --- Cargo.lock | 2 + crates/preflight/Cargo.toml | 4 + crates/preflight/src/lib.rs | 12 +-- crates/preflight/src/tests.rs | 46 ++++++++++++ crates/preflight/src/xmp.rs | 135 ++++++++++++++++++++++++++++++++++ parity/acrobat-features.toml | 2 +- 6 files changed, 194 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 53e0f097d5..68a758c2fb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4263,9 +4263,11 @@ dependencies = [ name = "pdfcraft-preflight" version = "0.9.0" dependencies = [ + "flate2", "pdfcraft-content", "pdfcraft-cos", "pdfcraft-model", + "roxmltree", ] [[package]] diff --git a/crates/preflight/Cargo.toml b/crates/preflight/Cargo.toml index b79575fc0b..082e44fc5c 100644 --- a/crates/preflight/Cargo.toml +++ b/crates/preflight/Cargo.toml @@ -14,3 +14,7 @@ workspace = true pdfcraft-content = { workspace = true } pdfcraft-cos = { workspace = true } pdfcraft-model = { workspace = true } +roxmltree = "0.20" + +[dev-dependencies] +flate2 = { workspace = true } diff --git a/crates/preflight/src/lib.rs b/crates/preflight/src/lib.rs index a4d90d874b..2cda29fea1 100644 --- a/crates/preflight/src/lib.rs +++ b/crates/preflight/src/lib.rs @@ -165,7 +165,7 @@ fn metadata(doc: &Document) -> Option { let cat = catalog(doc); let m = doc.resolve(cat.get(b"Metadata")?); match &*m { - Object::Stream(s) => s.decoded().ok().map(|b| String::from_utf8_lossy(&b).into_owned()), + Object::Stream(s) => s.decoded_within(xmp::MAX_XMP_BYTES).ok().map(|b| String::from_utf8_lossy(&b).into_owned()), _ => None, } } @@ -174,10 +174,10 @@ fn metadata(doc: &Document) -> Option { pub fn declared(doc: &Document) -> Declared { let mut d = Declared::default(); if let Some(x) = metadata(doc) { - let part = xmp::value(&x, "pdfaid:part").and_then(|p| p.parse().ok()); - let conf = xmp::value(&x, "pdfaid:conformance").unwrap_or_default(); + let part = xmp::identifier(&x, xmp::PDFA_ID, "part").and_then(|p| p.parse().ok()); + let conf = xmp::identifier(&x, xmp::PDFA_ID, "conformance").unwrap_or_default(); d.pdfa = part.map(|p| (p, conf.to_uppercase())); - d.pdfua = xmp::value(&x, "pdfuaid:part").and_then(|p| p.parse().ok()); + d.pdfua = xmp::identifier(&x, xmp::PDFUA_ID, "part").and_then(|p| p.parse().ok()); } let cat = catalog(doc); if let Some(list) = cat.get(b"OutputIntents").map(|o| doc.resolve(o)).and_then(|o| o.as_array().cloned()) { @@ -423,8 +423,8 @@ pub fn verify(doc: &Document, level: Level) -> Vec { match metadata(doc) { None => issue("6.6.2.1", "The document has no XMP metadata", None, None, true), Some(x) => { - let part = xmp::value(&x, "pdfaid:part"); - let conf = xmp::value(&x, "pdfaid:conformance"); + let part = xmp::identifier(&x, xmp::PDFA_ID, "part"); + let conf = xmp::identifier(&x, xmp::PDFA_ID, "conformance"); if part.as_deref() != Some(&level.part().to_string()) || conf.as_deref().map(str::to_uppercase).as_deref() != Some("B") { issue("6.6.4", "The metadata doesn't identify the file as {}", Some(level.label().to_owned()), None, true); } diff --git a/crates/preflight/src/tests.rs b/crates/preflight/src/tests.rs index 25d06669ed..80b1c63c49 100644 --- a/crates/preflight/src/tests.rs +++ b/crates/preflight/src/tests.rs @@ -371,3 +371,49 @@ fn device_colour_in_forms_patterns_and_glyphs_is_found() { assert!(declared(&doc).output_intents.is_empty(), "{what}: an sRGB output intent was added to a CMYK document: {:?}", r.fixed); } } + +#[test] +fn standards_identification_uses_namespace_not_prefix() { + fn with_xmp(xmp: &str) -> Document { + let stream = format!("<< /Type /Metadata /Subtype /XML /Length {} >>\nstream\n{xmp}\nendstream", xmp.len()); + build(&["<< /Type /Catalog /Pages 2 0 R /Metadata 3 0 R >>", "<< /Type /Pages /Kids [] /Count 0 >>", &stream], "") + } + let attribute = r#""#; + let element = r#"2B"#; + for xml in [attribute, element] { + let doc = with_xmp(xml); + assert_eq!(declared(&doc).pdfa, Some((2, "B".into()))); + assert!(!verify(&doc, Level::A2b).iter().any(|i| i.clause == "6.6.4")); + } + assert_eq!(declared(&with_xmp(attribute)).pdfua, Some(1)); + let wrong = r#""#; + let doc = with_xmp(wrong); + assert_eq!(declared(&doc).pdfa, None); + assert!(verify(&doc, Level::A2b).iter().any(|i| i.clause == "6.6.4")); + let nested = format!("{}{}", "".repeat(1100), "".repeat(1100)); + assert_eq!(declared(&with_xmp(&nested)).pdfa, None); +} + +#[test] +fn oversized_xmp_is_refused_while_decoding() { + let xmp = " ".repeat(xmp::MAX_XMP_BYTES + 1); + let stream = format!("<< /Type /Metadata /Subtype /XML /Length {} >>\nstream\n{xmp}\nendstream", xmp.len()); + let doc = build(&["<< /Type /Catalog /Pages 2 0 R /Metadata 3 0 R >>", "<< /Type /Pages /Kids [] /Count 0 >>", &stream], ""); + assert!(metadata(&doc).is_none()); + assert_eq!(declared(&doc).pdfa, None); + + use std::io::Write as _; + let mut z = flate2::write::ZlibEncoder::new(Vec::new(), flate2::Compression::default()); + z.write_all(xmp.as_bytes()).unwrap(); + let compressed = z.finish().unwrap(); + let hex: String = compressed.iter().map(|byte| format!("{byte:02X}")).collect(); + let data = format!("{hex}>"); + let stream = + format!("<< /Type /Metadata /Subtype /XML /Filter [/ASCIIHexDecode /FlateDecode] /Length {} >>\nstream\n{data}\nendstream", data.len()); + let doc = build(&["<< /Type /Catalog /Pages 2 0 R /Metadata 3 0 R >>", "<< /Type /Pages /Kids [] /Count 0 >>", &stream], ""); + let cat = catalog(&doc); + let raw = doc.resolve(cat.get(b"Metadata").unwrap()); + let Object::Stream(stream) = &*raw else { panic!("fixture has no metadata stream") }; + assert_eq!(stream.decoded().unwrap().len(), xmp::MAX_XMP_BYTES + 1, "fixture must actually decompress"); + assert!(metadata(&doc).is_none(), "compressed metadata must be limited during decoding"); +} diff --git a/crates/preflight/src/xmp.rs b/crates/preflight/src/xmp.rs index 3057213d1a..0da4869cc6 100644 --- a/crates/preflight/src/xmp.rs +++ b/crates/preflight/src/xmp.rs @@ -3,6 +3,116 @@ use pdfcraft_cos::Document; +/// The largest XMP packet read; a larger one is treated as missing. +pub(crate) const MAX_XMP_BYTES: usize = 8 * 1024 * 1024; +/// Namespace URIs of the PDF/A and PDF/UA identification schemas. +pub(crate) const PDFA_ID: &str = "http://www.aiim.org/pdfa/ns/id/"; +pub(crate) const PDFUA_ID: &str = "http://www.aiim.org/pdfua/ns/id/"; + +/// Read an identification property by its expanded XML name, not its arbitrary prefix. A packet +/// that is not well-formed XML (an undeclared prefix, an illegal character) or is too deep or wide +/// to parse safely is read by the conventional prefix (`pdfaid:`, `pdfuaid:`) instead, as before. +pub(crate) fn identifier(xmp: &str, namespace: &str, local: &str) -> Option { + if xmp.len() > MAX_XMP_BYTES { + return None; + } + match parsed_identifier(xmp, namespace, local) { + Ok(found) => found, + Err(()) => { + let prefix = match namespace { + PDFA_ID => "pdfaid", + PDFUA_ID => "pdfuaid", + _ => return None, + }; + value(xmp, &format!("{prefix}:{local}")) + } + } +} + +/// [`identifier`] through an XML parser; `Err` when the packet can't be parsed within the limits. +fn parsed_identifier(xmp: &str, namespace: &str, local: &str) -> Result, ()> { + if !xml_limits_ok(xmp) { + return Err(()); + } + let options = roxmltree::ParsingOptions { allow_dtd: false, nodes_limit: 100_000 }; + let doc = roxmltree::Document::parse_with_options(xmp, options).map_err(|_| ())?; + for node in doc.descendants().filter(roxmltree::Node::is_element) { + if let Some(value) = node.attribute((namespace, local)) { + return Ok(Some(value.trim().to_owned())); + } + if node.tag_name().namespace() == Some(namespace) && node.tag_name().name() == local { + return Ok(node.text().map(|value| value.trim().to_owned())); + } + } + Ok(None) +} + +/// Reject deep XML and start tags with too many attributes before roxmltree checks duplicates. +/// Each unquoted `=` in a valid start tag introduces an attribute, including namespace bindings. +/// Every `b[i]` and `hay[p..]` below is guarded by `i < b.len()` / `p < hay.len()`. +fn xml_limits_ok(text: &str) -> bool { + fn find(hay: &[u8], from: usize, needle: &[u8]) -> Option { + (from..hay.len()).find(|&p| hay[p..].starts_with(needle)) + } + let b = text.as_bytes(); + let (mut i, mut depth, mut total_attributes) = (0usize, 0usize, 0usize); + while i < b.len() { + if b[i] != b'<' { + i += 1; + } else if b[i..].starts_with(b"").map_or(b.len(), |p| p + 3); + } else if b[i..].starts_with(b"").map_or(b.len(), |p| p + 3); + } else if b[i..].starts_with(b"").map_or(b.len(), |p| p + 2); + } else if b[i..].starts_with(b"").map_or(b.len(), |p| p + 1); + } else if b[i..].starts_with(b"").map_or(b.len(), |p| p + 1); + } else { + depth += 1; + if depth > 1024 { + return false; + } + // Skip the rest of the start tag, over any quoted attribute value, and remember its last + // significant byte: a tag that ends in `/` closes itself, so it is one level deep while it + // is read rather than a level left open. + i += 1; + let mut quote = 0u8; + let mut last = 0u8; + let mut attributes = 0usize; + while i < b.len() { + let c = b[i]; + if quote != 0 { + if c == quote { + quote = 0; + } + } else if c == b'"' || c == b'\'' { + quote = c; + } else if c == b'>' { + i += 1; + break; + } else if c == b'=' { + attributes += 1; + total_attributes += 1; + if attributes > 128 || total_attributes > 4096 { + return false; + } + last = c; + } else if !c.is_ascii_whitespace() { + last = c; + } + i += 1; + } + if last == b'/' { + depth -= 1; + } + } + } + true +} + /// The value of a simple XMP property, written either as an attribute (`pdfaid:part="2"`) or /// as an element (`2`, or inside an `rdf:Alt`/`rdf:Seq`). pub fn value(xmp: &str, key: &str) -> Option { @@ -133,3 +243,28 @@ mod tests { assert_eq!(iso_date("junk"), None); } } + +#[cfg(test)] +mod limit_tests { + use super::*; + + #[test] + fn attribute_and_namespace_budgets_bound_parser_work() { + let prefix = r#"(); + let at_limit = format!("{prefix}{}/>", attrs(126)); + assert_eq!(identifier(&at_limit, PDFA_ID, "part").as_deref(), Some("2")); + let over_limit = format!("{prefix}{}/>", attrs(127)); + assert_eq!(identifier(&over_limit, PDFA_ID, "part"), None); + let namespaces = (0..129).map(|i| format!("xmlns:n{i}=\"urn:{i}\" ")).collect::(); + assert_eq!(identifier(&format!(""), PDFA_ID, "part"), None); + let many_elements = format!("{}", r#""#.repeat(4097)); + assert!(!xml_limits_ok(&many_elements)); + // Not well-formed (the prefix is never declared): read by the conventional prefix, as before. + let undeclared = r#""#; + assert_eq!(identifier(undeclared, PDFA_ID, "part").as_deref(), Some("3")); + assert_eq!(identifier(undeclared, PDFA_ID, "conformance").as_deref(), Some("u")); + let over_bytes = format!("{}", " ".repeat(8 * 1024 * 1024)); + assert_eq!(identifier(&over_bytes, PDFA_ID, "part"), None); + } +} diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 38e5a8d26e..94b2093b98 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -7776,7 +7776,7 @@ milestone = "M11" status = "partial" commands = ["standards.pdfa"] tools = ["pdfa_verify"] -evidence = ["crates/preflight/src/tests.rs::verify_reports_what_breaks_pdfa", "crates/preflight/src/tests.rs::convert_fixes_all_but_fonts", "crates/preflight/src/tests.rs::verify_reports_an_output_intent_that_does_not_match_device_colour", "crates/automation/tests/automation.rs::pdfa_through_tools", "crates/ui-egui/tests/compare.rs::pdfa_dialog_verifies_and_converts"] +evidence = ["crates/preflight/src/tests.rs::verify_reports_what_breaks_pdfa", "crates/preflight/src/tests.rs::standards_identification_uses_namespace_not_prefix", "crates/preflight/src/tests.rs::convert_fixes_all_but_fonts", "crates/preflight/src/tests.rs::verify_reports_an_output_intent_that_does_not_match_device_colour", "crates/automation/tests/automation.rs::pdfa_through_tools", "crates/ui-egui/tests/compare.rs::pdfa_dialog_verifies_and_converts"] notes = "PDF/A-2b/3b structural rules: identification, encryption, output intent (present, and RGB/CMYK to match DeviceRGB/DeviceCMYK use), fonts embedded, annotations, actions, images, embedded files. Not yet: content-stream operator checks, transparency details, PDF/A-1 and -4, a veraPDF comparison." [[feature]]