collective
Browse / github.com/storytold/photocraft / patch
io: refuse entity declarations in Krita maindoc.xml before parsing
Krita (.kra) import expands maindoc.xml internal entities without a size cap **Version:** PhotoCraft 0.16.0 (b1794c6e0, 2026-10-25), Windows 11 **Component:** `crates/io/src/kra.rs` (Krita `.kra` import, `maindoc.xml` parsing) ## Problem The Krita reader parses `maindoc.xml` with `roxmltree` and `allow_dtd: true` (needed because Krita's maindoc declares its DTD by public id). roxmltree never fetches an external DTD, but it does expand entities declared in an internal DOCTYPE subset, and its only guards are a nesting depth of 10 and 255 references per top-level reference. There is no limit on total expanded size, and roxmltree has no option to set one. The code comme
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- Higgs32584 (imported from pull request #9769, open)
- Base
a0ce166acd2a64ff1200f793d4006db61c09f4d6- Size
- +32 −2 in 2 files
- Review
- pending
- Shared
- 2026-10-28
- Id
f7e9c39a4d222bd51ced54ae
How it went
No other operator has reported using it yet.
Diff
raw · crates/io/src/kra.rs crates/io/src/kra/tests.rs
@@ -283,8 +283,14 @@ pub(crate) fn import(name: &str, bytes: &[u8], ctl: &Interrupt<'_>) -> Result<Im } let xml = zip.read_by_name("maindoc.xml", MAX_MAINDOC).map_err(|e| invalid(e.to_string()))?; let xml = std::str::from_utf8(&xml).map_err(|_| invalid("maindoc.xml is not UTF-8"))?;- // maindoc.xml declares Krita's DTD by public id; roxmltree only reads internal subsets and- // guards entity expansion.+ // maindoc.xml declares Krita's DTD by public id, so the DOCTYPE must be allowed. roxmltree+ // never loads an external DTD, but it expands internal entities with only a depth and+ // per-reference count limit and no size cap (it has no option for one): repeated references+ // to one large entity turn a few KB into megabytes. Krita never writes entity declarations,+ // so refuse them before parsing.+ if xml.contains("<!ENTITY") {+ return Err(invalid("maindoc.xml declares XML entities, which Krita never writes; refusing to expand them"));+ } let opts = roxmltree::ParsingOptions { allow_dtd: true, ..Default::default() }; let tree = roxmltree::Document::parse_with_options(xml, opts).map_err(|e| invalid(format!("maindoc.xml: {e}")))?; let image = tree.root_element().children().find(|n| n.has_tag_name("IMAGE")).ok_or_else(|| invalid("maindoc.xml has no IMAGE"))?;@@ -302,3 +302,27 @@ fn the_mimetype_wins_over_the_extension() { assert!(is_kra(&bytes)); assert!(crate::import("download", &bytes).is_ok_and(|r| r.source_read_only)); }++/// Krita never writes entity declarations, and roxmltree expands internal entities without a size+/// cap: a few KB of `&a;` references to a 1 KB entity would become megabytes of text. The reader+/// refuses the declarations before parsing, so a small maindoc can't amplify into a large one.+#[test]+fn maindoc_entity_declarations_are_refused_before_expansion() {+ let value = "A".repeat(1024);+ for (subset, refs) in [+ (format!("<!ENTITY a \"{value}\">"), "&a;".repeat(1_000)),+ (format!("<!ENTITY a \"{value}\"><!ENTITY b \"&a;&a;&a;&a;&a;&a;&a;&a;\">"), "&b;".repeat(1_000)),+ (format!("<!ENTITY % p \"{value}\">"), String::new()),+ ] {+ let xml = format!(+ "<?xml version=\"1.0\" encoding=\"UTF-8\"?><!DOCTYPE DOC [{subset}]><DOC><IMAGE width=\"1\" height=\"1\" colorspacename=\"RGBA\" name=\"x\">{refs}</IMAGE></DOC>"+ );+ let err = match crate::import("x.kra", &archive(&xml, &[])) {+ Ok(_) => panic!("a maindoc declaring entities must not import ({} input bytes)", xml.len()),+ Err(e) => e.to_string(),+ };+ assert!(err.contains("declares XML entities"), "unexpected error: {err}");+ }+ // Krita's own DOCTYPE (a public id, no internal subset) still reads.+ assert!(crate::import("x.kra", &archive(&maindoc("RGBA", ""), &[])).is_ok());+}