From 8e692d83ed1464a61b935e59e0b65c67672ccac0 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Mon, 2 Nov 2026 20:29:45 -0500 Subject: [PATCH 1/2] ICC: refuse a profile whose tag table lists a tag twice moxcms parses every entry of the tag table, so a profile listing one description, copyright or table tag thousands of times, each pointing at the same large data, made it decode and copy that data once per entry. The ICC specification allows each tag signature once; such a table is now refused before parsing. --- crates/color/src/cms/icc.rs | 20 ++++++++++++++++++++ crates/color/src/cms/tests_icc.rs | 26 ++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/crates/color/src/cms/icc.rs b/crates/color/src/cms/icc.rs index 64f42f25b..2599956ce 100644 --- a/crates/color/src/cms/icc.rs +++ b/crates/color/src/cms/icc.rs @@ -106,6 +106,9 @@ impl IccProfile { if !localized_text_fits(bytes) { return Err(CmsError::BadProfile(format!("its localized names take more than {} KB", MAX_LOCALIZED_TEXT >> 10))); } + if let Some(tag) = repeated_tag(bytes) { + return Err(CmsError::BadProfile(format!("its tag table lists the `{}` tag more than once", String::from_utf8_lossy(&tag.to_be_bytes())))); + } let p = ColorProfile::new_from_slice(bytes).map_err(|e| CmsError::BadProfile(format!("{e:?}")))?; Ok(Self { source: bytes.to_vec(), ..Self::from_profile(name, p)? }) } @@ -417,6 +420,23 @@ pub(super) fn gray_profile(name: &str) -> ColorProfile { labelled(p, name) } +/// The first tag signature the tag table of ICC `bytes` lists twice, which the ICC +/// specification forbids. moxcms parses every entry of the table, so a profile listing one +/// description, copyright or table tag thousands of times, each pointing at the same large data, +/// would decode and copy that data once per entry. A table it can't read is left to moxcms. +pub(crate) fn repeated_tag(bytes: &[u8]) -> Option { + let be32 = |at: usize| bytes.get(at..at.checked_add(4)?).map(|v| u32::from_be_bytes([v[0], v[1], v[2], v[3]])); + let tags = be32(128)? as usize; + let mut seen = std::collections::HashSet::new(); + for i in 0..tags { + let sig = be32(i.checked_mul(12)?.checked_add(132)?)?; + if !seen.insert(sig) { + return Some(sig); + } + } + None +} + /// Most bytes of UTF-16 the localized names (`mluc` tags) of a profile may take together. const MAX_LOCALIZED_TEXT: usize = 1 << 20; diff --git a/crates/color/src/cms/tests_icc.rs b/crates/color/src/cms/tests_icc.rs index 46e44cd76..1d312c09c 100644 --- a/crates/color/src/cms/tests_icc.rs +++ b/crates/color/src/cms/tests_icc.rs @@ -181,6 +181,32 @@ fn aliased_localized_names_are_refused() { assert!(!super::icc::localized_text_fits(&profile(4096, 64 << 10))); } +/// A tag table that lists one tag many times, every entry pointing at the same long text, is +/// refused before moxcms decodes the text once per entry; distinct tags pass. +#[test] +fn repeated_tags_are_refused() { + let profile = |sigs: &[&[u8; 4]], text: usize| { + let mut b = vec![0u8; 128]; + b.extend((sigs.len() as u32).to_be_bytes()); + let tag_at = 128 + 4 + 12 * sigs.len(); + let size = 8 + text; + for sig in sigs { + b.extend(*sig); + b.extend((tag_at as u32).to_be_bytes()); + b.extend((size as u32).to_be_bytes()); + } + b.extend(b"text"); + b.extend([0; 4]); + b.extend(vec![b'a'; text]); + b + }; + assert_eq!(super::icc::repeated_tag(&profile(&[b"desc", b"cprt", b"dmdd"], 64)), None); + let many: Vec<&[u8; 4]> = std::iter::repeat_n(b"desc", 4096).collect(); + let bytes = profile(&many, 256 << 10); + assert_eq!(super::icc::repeated_tag(&bytes), Some(u32::from_be_bytes(*b"desc"))); + assert!(matches!(super::icc::IccProfile::from_bytes(None, &bytes), Err(super::CmsError::BadProfile(_)))); +} + /// Reloading an RGB profile under the name it had (a new file for "My RGB") changes what CMYK /// converts to in it: the direct CMYK → RGB transform isn't kept from the old file. #[test] From 1bd47ea30d9e7a2909b6094a129632a96e93f6ff Mon Sep 17 00:00:00 2001 From: bflatastic Date: Tue, 3 Nov 2026 15:58:07 +0700 Subject: [PATCH 2/2] Format with rustfmt --- crates/color/src/cms/icc.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/color/src/cms/icc.rs b/crates/color/src/cms/icc.rs index 2599956ce..0b09062b8 100644 --- a/crates/color/src/cms/icc.rs +++ b/crates/color/src/cms/icc.rs @@ -107,7 +107,10 @@ impl IccProfile { return Err(CmsError::BadProfile(format!("its localized names take more than {} KB", MAX_LOCALIZED_TEXT >> 10))); } if let Some(tag) = repeated_tag(bytes) { - return Err(CmsError::BadProfile(format!("its tag table lists the `{}` tag more than once", String::from_utf8_lossy(&tag.to_be_bytes())))); + return Err(CmsError::BadProfile(format!( + "its tag table lists the `{}` tag more than once", + String::from_utf8_lossy(&tag.to_be_bytes()) + ))); } let p = ColorProfile::new_from_slice(bytes).map_err(|e| CmsError::BadProfile(format!("{e:?}")))?; Ok(Self { source: bytes.to_vec(), ..Self::from_profile(name, p)? })