fix: validate XML attribute identities and preflight GIF frame limits

crates/engine/src/cmd.rs

@@ -97,6 +97,8 @@ pub(crate) mod symboledit;
 pub mod symbollib;
 pub(crate) mod tabs;
 mod textedit;
+mod xmlattrs;
+
 pub use textedit::effective_kerning;
 pub mod textstyles;
 pub(crate) mod textwrap;

crates/engine/src/cmd/place/docx.rs

@@ -167,10 +167,13 @@ fn parse(xml: &str, name: &str, cmd: &str) -> Result<El> {
         ..Options::default()
     }
     .strict();
-    opts.parse(xml.as_bytes()).map_err(|e| match e {
+    let doc = opts.parse(xml.as_bytes()).map_err(|e| match e {
         craft_xml::Error::TooDeep(_) => bad(cmd, format!("`{name}` in the Word document nests elements more than {MAX_NESTING} deep")),
         _ => bad(cmd, format!("`{name}` in the Word document is not well-formed XML: {e}")),
-    })
+    })?;
+    crate::cmd::xmlattrs::validate(xml.as_bytes(), opts.limits)
+        .map_err(|e| bad(cmd, format!("`{name}` in the Word document is not well-formed XML: {e}")))?;
+    Ok(doc)
 }
 
 /// The part that the first relationship of part `source` (`""`: the package) whose type ends in

crates/engine/src/cmd/variablecmds/library.rs

@@ -199,7 +199,7 @@ impl Library {
 fn parse_library(xml: &str, cmd: &str) -> Result<Library> {
     let xml = without_doctype(xml, cmd)?;
     let opts = craft_xml::Options {
-        names: craft_xml::Names::Local,
+        names: craft_xml::Names::AsWritten,
         limits: craft_xml::Limits { max_depth: MAX_LIBRARY_NESTING, max_nodes: MAX_LIBRARY_NODES as usize, ..craft_xml::Limits::DEFAULT },
         ..craft_xml::Options::default()
     }
@@ -208,12 +208,13 @@ fn parse_library(xml: &str, cmd: &str) -> Result<Library> {
         craft_xml::Error::TooDeep(_) => bad(cmd, format!("the variable library nests elements more than {MAX_LIBRARY_NESTING} deep")),
         _ => bad(cmd, format!("the variable library is not well-formed XML: {e}")),
     })?;
-    let Some(root) = std::iter::once(&doc).chain(doc.descendants()).find(|n| n.is("variableSets")) else {
+    crate::cmd::xmlattrs::validate(xml.as_bytes(), opts.limits).map_err(|e| bad(cmd, format!("the variable library is not well-formed XML: {e}")))?;
+    let Some(root) = std::iter::once(&doc).chain(doc.descendants()).find(|n| n.is(craft_xml::Local("variableSets"))) else {
         return Err(bad(cmd, "the file holds no `variableSets` element"));
     };
-    let sets: Vec<&craft_xml::El> = root.children("variableSet").collect();
+    let sets: Vec<&craft_xml::El> = root.children(craft_xml::Local("variableSet")).collect();
     let (mut text, mut other, mut declared, mut refs) = (Vec::new(), Vec::new(), BTreeSet::new(), Vec::new());
-    for var in sets.iter().flat_map(|s| s.children("variables")).flat_map(|v| v.children("variable")) {
+    for var in sets.iter().flat_map(|s| s.children(craft_xml::Local("variables"))).flat_map(|v| v.children(craft_xml::Local("variable"))) {
         let name = named(var.attr("varName"), "varName", cmd)?;
         if !declared.insert(name.clone()) {
             return Err(bad(cmd, format!("variable `{name}` is declared twice")));
@@ -239,7 +240,7 @@ fn parse_library(xml: &str, cmd: &str) -> Result<Library> {
     }
     let text_names: BTreeSet<&str> = text.iter().map(String::as_str).collect();
     let (mut datasets, mut seen) = (Vec::new(), BTreeSet::new());
-    for row in sets.iter().flat_map(|s| s.children("sampleDataSets")).flat_map(|d| d.children("sampleDataSet")) {
+    for row in sets.iter().flat_map(|s| s.children(craft_xml::Local("sampleDataSets"))).flat_map(|d| d.children(craft_xml::Local("sampleDataSet"))) {
         let name = named(row.attr("dataSetName"), "dataSetName", cmd)?;
         if !seen.insert(name.clone()) {
             return Err(bad(cmd, format!("data set `{name}` is declared twice")));
@@ -249,7 +250,7 @@ fn parse_library(xml: &str, cmd: &str) -> Result<Library> {
         }
         let (mut values, mut held) = (BTreeMap::new(), BTreeSet::new());
         for el in row.elements() {
-            let var = el.name.as_str();
+            let var = el.local();
             if !declared.contains(var) {
                 return Err(bad(cmd, format!("data set `{name}` holds a value for `{var}`, which the library doesn't declare")));
             }
@@ -259,7 +260,7 @@ fn parse_library(xml: &str, cmd: &str) -> Result<Library> {
             if !text_names.contains(var) {
                 continue;
             }
-            let paras: Vec<String> = el.children("p").map(craft_xml::El::deep_text).collect();
+            let paras: Vec<String> = el.children(craft_xml::Local("p")).map(craft_xml::El::deep_text).collect();
             if !paras.is_empty() {
                 values.insert(var.to_string(), DataValue::Text(paras.join("\n")));
             }

crates/engine/src/cmd/xmlattrs.rs

@@ -0,0 +1,55 @@
+//! Attribute uniqueness checks missing from the pinned shared XML reader's strict mode.
+//!
+//! Temporary consumer guard: parsing remains in craft-xml. Move this invariant into the
+//! shared strict reader and remove this second event pass when that dependency is updated.
+
+/// Check names before canonical/local-name rewriting can collapse distinct attributes.
+/// The event reader and namespace scope use the same caller-supplied resource caps.
+pub(super) fn validate(xml: &[u8], limits: craft_xml::Limits) -> Result<(), String> {
+    use std::collections::BTreeSet;
+
+    let opts = craft_xml::Options { limits, doctype: craft_xml::Doctype::Reject, ..Default::default() }.strict();
+    let mut events = opts.events(xml);
+    let mut bindings: Vec<(String, String)> = Vec::new();
+    let mut scopes = Vec::new();
+    while let Some(event) = events.next().map_err(|e| e.to_string())? {
+        match event {
+            craft_xml::Event::Start { attrs, .. } => {
+                scopes.push(bindings.len());
+                let mut raw = BTreeSet::new();
+                for (key, value) in &attrs {
+                    if !raw.insert(key.as_str()) {
+                        return Err(format!("duplicate attribute `{key}`"));
+                    }
+                    if let Some(prefix) = key.strip_prefix("xmlns:") {
+                        if bindings.len() >= limits.max_attrs {
+                            return Err("too many in-scope namespace declarations".into());
+                        }
+                        bindings.push((prefix.to_string(), value.clone()));
+                    }
+                }
+                let mut expanded = BTreeSet::new();
+                for (key, _) in &attrs {
+                    if key == "xmlns" || key.starts_with("xmlns:") {
+                        continue;
+                    }
+                    // Default namespaces do not apply to attributes. Preserve undeclared
+                    // prefixes for standalone SVG library fragments, as the importer did.
+                    let name = match key.split_once(':') {
+                        Some(("xml", local)) => ("http://www.w3.org/XML/1998/namespace", local),
+                        Some((prefix, local)) => {
+                            bindings.iter().rev().find(|(p, _)| p == prefix).map_or(("", key.as_str()), |(_, uri)| (uri.as_str(), local))
+                        }
+                        None => ("", key.as_str()),
+                    };
+                    if !expanded.insert(name) {
+                        return Err(format!("duplicate expanded attribute `{key}`"));
+                    }
+                }
+            }
+            craft_xml::Event::End { .. } => bindings.truncate(scopes.pop().unwrap_or(0)),
+            _ => {}
+        }
+    }
+    Ok(())
+}

crates/engine/src/tests/textimport.rs

@@ -471,6 +471,8 @@ fn malformed_docx_fails_cleanly() {
     for xml in [
         format!(r#"<w:document xmlns:w="{W_NS}"><w:body><w:p><w:r><w:t>partial text</w:t></w:r></w:p>"#),
         document("<w:p><w:r><w:t>A&missing;B</w:t></w:r></w:p>"),
+        document("<w:p><w:r><w:t xml:space=\"preserve\" xml:space=\"default\">text</w:t></w:r></w:p>"),
+        document("<w:p xmlns:a=\"urn:test\" xmlns:b=\"urn:test\" a:id=\"1\" b:id=\"2\"><w:r><w:t>text</w:t></w:r></w:p>"),
     ] {
         let e = place(&mut s, "a.docx", &store(&[("word/document.xml", xml.as_bytes())])).unwrap_err();
         assert!(e.contains("not well-formed XML"), "{e}");

crates/engine/src/tests/variables.rs

@@ -2481,6 +2481,9 @@ fn malformed_variable_libraries_are_refused() {
         ("not well-formed XML", "<variableSets><variableSet>".to_string()),
         ("not well-formed XML", ok.replace("<p>x</p>", "<p>A&missing;B</p>")),
         ("not well-formed XML", ok.replace("</variableSets>", "")),
+        ("not well-formed XML", ok.replace("varName=\"Name\"", "varName=\"Name\" varName=\"Other\"")),
+        ("not well-formed XML", ok.replace("<variable ", "<variable xmlns:a=\"urn:test\" xmlns:b=\"urn:test\" a:id=\"1\" b:id=\"2\" ")),
+        ("missing `varName`", ok.replace("varName=\"Name\"", "xmlns:m=\"urn:metadata\" m:varName=\"Name\"")),
         ("no `variableSets` element", "<svg><variables/></svg>".to_string()),
         ("declares no variables", "<variableSets><variableSet><variables/></variableSet></variableSets>".to_string()),
         ("is declared twice", library(&[("Name", "textcontent"), ("Name", "textcontent")], &[])),
@@ -3174,3 +3177,14 @@ fn an_svg_without_readable_variable_data_opens_its_art() {
         assert_eq!(vectorcraft_testkit::svg_shapes(&vectorcraft_svg::export(d, &Default::default())), 1, "the art opens");
     }
 }
+
+#[test]
+fn variable_library_attributes_keep_their_namespace() {
+    let mut s = session();
+    let xml = library(&[("Name", "textcontent")], &[("A", &[("Name", &["x"])])])
+        .replace("varName=\"Name\"", "xmlns:m=\"urn:metadata\" m:varName=\"Other\" varName=\"Name\"");
+    let loaded = s.execute("variable.loadLibrary", &json!({"xml": xml})).unwrap();
+    assert_eq!(loaded["variables"], json!(["Name"]));
+    let sets = s.execute("dataset.list", &json!({})).unwrap();
+    assert_eq!(sets["datasets"][0]["values"], json!({"Name": {"text": "x"}}));
+}

crates/trace/src/raster.rs

@@ -35,6 +35,11 @@ pub const MAX_SOURCE_PIXELS: u64 = 192 << 20;
 pub(crate) fn trace_header(bytes: &[u8], max_pixels: u64, max_alloc: u64) -> Result<Option<craft_image_codecs::RasterInspection>, TraceError> {
     use craft_image_codecs::{ChannelLayout, Format, Limits, PngDecodeRows, RasterColorType, RasterInspection};
     let decode_err = |e: craft_image_codecs::CodecError| TraceError::Decode(e.to_string());
+    // Refuse a GIF frame larger than its logical screen before inspection's
+    // plausibility check can turn the caller's size refusal into a decode error.
+    if gif_first_frame_pixels(bytes).is_some_and(|pixels| pixels > max_pixels) {
+        return Err(TraceError::TooLarge { width: 0, height: 0, max: max_pixels >> 20 });
+    }
     if craft_image_codecs::detect(bytes) != Some(Format::Png) {
         return craft_image_codecs::inspect_raster(bytes).map_err(decode_err);
     }

crates/trace/src/tests.rs

@@ -3973,6 +3973,7 @@ fn a_gif_frame_larger_than_its_screen_is_refused_before_decoding() {
     gif.extend([2, 1, 0, 0, 0x3B]);
     assert!(matches!(Raster::decode(&gif), Err(TraceError::TooLarge { .. })), "{:?}", Raster::decode(&gif).err());
     assert!(matches!(Raster::decode_to_fit(&gif), Err(TraceError::TooLarge { .. })));
+    assert!(matches!(decode_alpha(&gif), Err(TraceError::TooLarge { .. })));
 }
 
 // Synthetic PNG chunks for transparency/orientation regression fixtures.