From a3ec06580cc112b351ebab845a7997e1bf502929 Mon Sep 17 00:00:00 2001 From: skylar taylor-barrick Date: Mon, 19 Oct 2026 12:50:32 -0600 Subject: [PATCH] fix(redact): preserve OCMD visibility during sanitization --- crates/redact/src/interp.rs | 144 ++++++++++++++++++++++++++-------- crates/redact/src/sanitize.rs | 32 +++++--- crates/redact/src/tests.rs | 121 ++++++++++++++++++++++++++++ 3 files changed, 252 insertions(+), 45 deletions(-) diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index 7417f05a0d..94b4a36c94 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -84,9 +84,16 @@ pub(crate) struct Scope<'a> { pub report: &'a mut Report, /// Sanitize: remove hidden text (invisible render modes, or wholly outside this page box). pub hidden_text: Option<[f64; 4]>, - /// Sanitize: optional content groups (and membership dictionaries) that are off; content - /// marked with them is removed. - pub hidden_layers: Vec, + /// Sanitize: optional content groups that are off; content marked with them is removed. + pub hidden_layers: HashSet, + /// Whether this pass is evaluating optional-content rules. An empty off set is meaningful: + /// policies such as `/AllOff` can still hide content when every group is on. + pub layers_enabled: bool, + /// Off groups referenced by optional content that remains visible and therefore still needs + /// its original default state after sanitization. + pub retained_off_layers: HashSet, + /// Cache membership results so repeated blocks cannot repeatedly walk hostile expressions. + ocmd_cache: HashMap, /// Content removed from hidden layers (blocks and XObjects). pub layer_blocks: usize, fonts: HashMap, Rc>, @@ -101,7 +108,10 @@ impl<'a> Scope<'a> { mode, report, hidden_text: None, - hidden_layers: Vec::new(), + hidden_layers: HashSet::new(), + layers_enabled: false, + retained_off_layers: HashSet::new(), + ocmd_cache: HashMap::new(), layer_blocks: 0, fonts: HashMap::new(), used_names: Vec::new(), @@ -111,18 +121,23 @@ impl<'a> Scope<'a> { /// Sanitizing visits every form XObject (not only those under a region). fn everywhere(&self) -> bool { - self.hidden_text.is_some() || !self.hidden_layers.is_empty() + self.hidden_text.is_some() || self.layers_enabled } /// Is this optional-content reference (an OCG or OCMD) hidden? - fn layer_hidden(&self, doc: &Document, o: &Object) -> bool { - if self.hidden_layers.is_empty() { + fn layer_hidden(&mut self, doc: &Document, o: &Object) -> bool { + if !self.layers_enabled { return false; } - if let Some(r) = o.as_ref() - && self.hidden_layers.contains(&r) - { - return true; + let reference = o.as_ref(); + if let Some(r) = reference { + if let Some(hidden) = self.ocmd_cache.get(&r) { + return *hidden; + } + if self.hidden_layers.contains(&r) { + self.ocmd_cache.insert(r, true); + return true; + } } // Evaluate the policy against the same default configuration used to select hidden // groups. A membership dictionary's default policy is AnyOn. @@ -137,29 +152,92 @@ impl<'a> Scope<'a> { None => Vec::new(), }; let fail_closed = || groups.iter().any(|g| self.hidden_layers.contains(g)); - if let Some(expression) = d.get(b"VE") { + let hidden = if let Some(expression) = d.get(b"VE") { let visible = self.visibility_expression(doc, expression, 0, &mut HashSet::new(), &mut 4096); - return !visible - .unwrap_or_else(|| !fail_closed() && !self.expression_mentions_hidden(doc, expression, 0, &mut HashSet::new(), &mut 4096)); + match visible { + Some(visible) => !visible, + None => fail_closed() || self.expression_mentions_hidden(doc, expression, 0, &mut HashSet::new(), &mut 4096), + } + } else if groups.is_empty() { + false + } else { + let any_on = groups.iter().any(|g| !self.hidden_layers.contains(g)); + let all_on = groups.iter().all(|g| !self.hidden_layers.contains(g)); + let policy = d.get(b"P").map(|p| doc.resolve(p)).and_then(|p| p.as_name().map(|name| name.to_vec())); + match (d.get(b"P"), policy.as_deref()) { + (None, _) | (Some(_), Some(b"AnyOn")) => !any_on, + (Some(_), Some(b"AllOn")) => !all_on, + (Some(_), Some(b"AnyOff")) => all_on, + (Some(_), Some(b"AllOff")) => any_on, + // An invalid policy must not make a candidate hidden block survive sanitization. + _ => fail_closed(), + } + }; + if !hidden { + self.retained_off_layers.extend(groups.iter().filter(|g| self.hidden_layers.contains(g)).copied()); + if let Some(expression) = d.get(b"VE") { + let mut referenced = HashSet::new(); + if Self::collect_hidden_expression_groups(doc, expression, &self.hidden_layers, 0, &mut HashSet::new(), &mut 4096, &mut referenced) { + self.retained_off_layers.extend(referenced); + } else { + // If an expression cannot be fully walked, preserve every off group so that + // the content we kept does not change visibility after configuration cleanup. + self.retained_off_layers.extend(self.hidden_layers.iter().copied()); + } + } } - if groups.is_empty() { + if let Some(r) = reference { + self.ocmd_cache.insert(r, hidden); + } + hidden + } + + fn collect_hidden_expression_groups( + doc: &Document, + expression: &Object, + hidden_layers: &HashSet, + depth: usize, + path: &mut HashSet, + budget: &mut usize, + found: &mut HashSet, + ) -> bool { + if depth > 64 || *budget == 0 { return false; } - let any_on = groups.iter().any(|g| !self.hidden_layers.contains(g)); - let all_on = groups.iter().all(|g| !self.hidden_layers.contains(g)); - let policy = match d.get(b"P") { - None => b"AnyOn".as_slice(), - Some(Object::Name(name)) => name.as_slice(), - Some(_) => return fail_closed(), - }; - match policy { - b"AnyOn" => !any_on, - b"AllOn" => !all_on, - b"AnyOff" => all_on, - b"AllOff" => any_on, - // An invalid policy must not make a candidate hidden block survive sanitization. - _ => fail_closed(), + *budget -= 1; + let reference = expression.as_ref(); + if let Some(r) = reference + && !path.insert(r) + { + return false; } + let resolved = doc.resolve(expression); + let complete = match &*resolved { + Object::Dict(d) if d.name(b"Type") == Some(b"OCG") => { + if let Some(r) = reference + && hidden_layers.contains(&r) + { + found.insert(r); + } + reference.is_some() + } + Object::Array(items) => { + let Some(Object::Name(operator)) = items.first() else { return false }; + let operands = match operator.as_slice() { + b"And" | b"Or" if (2..=4097).contains(&items.len()) => items.get(1..), + b"Not" if items.len() == 2 => items.get(1..), + _ => None, + }; + operands.is_some_and(|operands| { + operands.iter().all(|item| Self::collect_hidden_expression_groups(doc, item, hidden_layers, depth + 1, path, budget, found)) + }) + } + _ => false, + }; + if let Some(r) = reference { + path.remove(&r); + } + complete } /// Evaluate an optional-content visibility expression (`/VE`) against the default config. @@ -191,14 +269,14 @@ impl<'a> Scope<'a> { _ => return None, }; match operator { - b"And" if items.len() >= 3 => { + b"And" if items.len() >= 2 => { let mut visible = true; for item in items.iter().skip(1) { visible &= self.visibility_expression(doc, item, depth + 1, path, budget)?; } Some(visible) } - b"Or" if items.len() >= 3 => { + b"Or" if items.len() >= 2 => { let mut visible = false; for item in items.iter().skip(1) { visible |= self.visibility_expression(doc, item, depth + 1, path, budget)?; @@ -641,7 +719,7 @@ pub(crate) fn process(doc: &mut Document, scope: &mut Scope<'_>, streams: &[Vec< // Layer removal also changes enclosing replacement spans, so remember the operators before // any cleanup, not just the survivors of the layer pass. let original = parsed.ops.clone(); - if !scope.hidden_layers.is_empty() { + if scope.layers_enabled { let props = res_dict(doc, resources, b"Properties"); let (kept, removed) = strip_hidden_layers(doc, scope, parsed.ops, &props); parsed.ops = kept; @@ -1180,7 +1258,7 @@ fn xobject( /// Remove marked-content blocks of hidden layers (`/OC /name BDC … EMC`, nested blocks /// included). Returns the remaining operators and how many blocks went. -fn strip_hidden_layers(doc: &Document, scope: &Scope<'_>, ops: Vec, props: &Dict) -> (Vec, usize) { +fn strip_hidden_layers(doc: &Document, scope: &mut Scope<'_>, ops: Vec, props: &Dict) -> (Vec, usize) { let mut out = Vec::with_capacity(ops.len()); // For each open marked-content block: does it hide its contents? let mut stack: Vec = Vec::new(); diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index 1940e376a8..bafa769bd3 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -138,12 +138,13 @@ fn hidden_layers(doc: &Document) -> Vec { } /// Glyphs of hidden text, or content of hidden layers, on every page (`write` = remove them). -fn content_pass(doc: &mut Document, text: bool, layers: bool, write: bool) -> Result { +fn content_pass(doc: &mut Document, text: bool, layers: bool, write: bool) -> Result<(usize, std::collections::HashSet), RedactError> { let off = if layers { hidden_layers(doc) } else { Vec::new() }; - if !text && off.is_empty() { - return Ok(0); + if !text && !layers { + return Ok((0, std::collections::HashSet::new())); } let mut total = 0; + let mut retained_off_layers = std::collections::HashSet::new(); // The original image and form XObjects the rewrite drops or replaces, on any page. let mut doomed = Vec::new(); let mut retired_properties = Vec::new(); @@ -164,13 +165,18 @@ fn content_pass(doc: &mut Document, text: bool, layers: bool, write: bool) -> Re let c = page.crop(doc); scope.hidden_text = Some([c[0].min(c[2]), c[1].min(c[3]), c[0].max(c[2]), c[1].max(c[3])]); } - scope.hidden_layers = off.clone(); + scope.hidden_layers = off.iter().copied().collect(); + scope.layers_enabled = layers; let out = process(doc, &mut scope, &data, &resources, Matrix::IDENTITY); if let Some(error) = out.error { return Err(error.into()); } let blocks = scope.layer_blocks; - total += out.residue + blocks + report.glyphs; + let glyphs = scope.report.glyphs; + let retained_for_page = std::mem::take(&mut scope.retained_off_layers); + drop(scope); + total += out.residue + blocks + glyphs; + retained_off_layers.extend(retained_for_page); if !write { continue; } @@ -216,7 +222,7 @@ fn content_pass(doc: &mut Document, text: bool, layers: bool, write: bool) -> Re // What a page no longer draws must not stay reachable through an inherited or shared // /Resources either. crate::interp::unlink_doomed(doc, &doomed, &retired_properties)?; - Ok(total) + Ok((total, retained_off_layers)) } /// How many items each category would remove (categories with nothing are included as 0). @@ -239,10 +245,10 @@ pub fn scan(doc: &Document) -> Vec<(Hidden, usize)> { Hidden::Attachments => sub(doc, &names, b"EmbeddedFiles").map_or(0, |t| name_tree_len(doc, &t, 0)) + count_sub(b"FileAttachment"), Hidden::Comments => annots.iter().filter(|a| is_comment(a.name(b"Subtype").unwrap_or(b""))).count(), Hidden::FormFields => pdfcraft_forms::fields(doc).len(), - Hidden::HiddenText => content_pass(&mut doc2, true, false, false).unwrap_or(0), + Hidden::HiddenText => content_pass(&mut doc2, true, false, false).map(|(n, _)| n).unwrap_or(0), Hidden::HiddenLayers => { let off = hidden_layers(doc).len(); - if off == 0 { 0 } else { off + content_pass(&mut doc2, false, true, false).unwrap_or(0) } + off + content_pass(&mut doc2, false, true, false).map(|(n, _)| n).unwrap_or(0) } Hidden::Bookmarks => sub(doc, &cat, b"Outlines").map_or(0, |o| outline_len(doc, o.get(b"First"), 0)), Hidden::LinksActionsScripts => { @@ -274,9 +280,11 @@ pub fn remove_hidden(doc: &mut Document, which: &[Hidden]) -> Result 0) || (layers && count(Hidden::HiddenLayers) > 0) { - content_pass(doc, text, layers, true)?; - } + let retained_off_layers = if (text && count(Hidden::HiddenText) > 0) || (layers && count(Hidden::HiddenLayers) > 0) { + content_pass(doc, text, layers, true)?.1 + } else { + std::collections::HashSet::new() + }; // Every annotation the pages list now: whatever is no longer listed at the end (flattened // widgets included) leaves the structure tree too. let listed = |doc: &Document| -> std::collections::HashSet { @@ -394,7 +402,7 @@ pub fn remove_hidden(doc: &mut Document, which: &[Hidden]) -> Result { // The off layers leave the configuration (their content is gone already). if let Some(mut oc) = sub(doc, &cat, b"OCProperties") { - let keep = |o: &Object| !o.as_ref().is_some_and(|r| off.contains(&r)); + let keep = |o: &Object| !o.as_ref().is_some_and(|r| off.contains(&r) && !retained_off_layers.contains(&r)); if let Some(Object::Array(a)) = oc.get(b"OCGs").map(|g| (*doc.resolve(g)).clone()) { oc.set(b"OCGs".to_vec(), Object::Array(a.into_iter().filter(keep).collect())); } diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index d6931116c2..421c1a557a 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -569,6 +569,116 @@ fn hidden_ocmd_content_in_a_form_and_nested_marked_blocks_is_removed() { assert!(!pdfcraft_edit::text_lines(&saved, 0).unwrap().iter().any(|line| line.text.contains("SECRET"))); } +#[test] +fn hidden_ocmd_alloff_is_sanitized_when_every_group_is_on() { + use crate::sanitize::{Hidden, remove_hidden, scan}; + + let mut doc = one_page( + b"/OC /M BDC BT /F1 10 Tf 10 100 Td (SECRET) Tj ET EMC", + "/Properties << /M 9 0 R >>", + vec![ + b"<< /Type /OCG /Name (On) >>".to_vec(), + b"<< /Type /OCG /Name (Unused) >>".to_vec(), + b"<< /Type /OCMD /OCGs [7 0 R] /P /AllOff >>".to_vec(), + ], + ); + let counts = scan(&doc); + assert!(counts.iter().any(|(h, n)| *h == Hidden::HiddenLayers && *n > 0), "{counts:?}"); + remove_hidden(&mut doc, &[Hidden::HiddenLayers]).unwrap(); + + let saved = Document::open(Arc::new(write_full(&doc, &SaveOptions::default()).unwrap())).unwrap(); + assert!(!pdfcraft_edit::text_lines(&saved, 0).unwrap().iter().any(|line| line.text.contains("SECRET"))); +} + +#[test] +fn hidden_ve_not_is_sanitized_when_its_group_is_on() { + use crate::sanitize::{Hidden, remove_hidden, scan}; + + let mut doc = one_page( + b"/OC /M BDC BT /F1 10 Tf 10 100 Td (SECRET) Tj ET EMC", + "/Properties << /M 9 0 R >>", + vec![b"<< /Type /OCG /Name (On) >>".to_vec(), b"<< /Type /OCG /Name (Unused) >>".to_vec(), b"<< /Type /OCMD /VE [/Not 7 0 R] >>".to_vec()], + ); + let counts = scan(&doc); + assert!(counts.iter().any(|(h, n)| *h == Hidden::HiddenLayers && *n > 0), "{counts:?}"); + remove_hidden(&mut doc, &[Hidden::HiddenLayers]).unwrap(); + + let saved = Document::open(Arc::new(write_full(&doc, &SaveOptions::default()).unwrap())).unwrap(); + assert!(!pdfcraft_edit::text_lines(&saved, 0).unwrap().iter().any(|line| line.text.contains("SECRET"))); +} + +#[test] +fn hidden_ocmd_resolves_an_indirect_policy_name() { + use crate::sanitize::{Hidden, remove_hidden}; + + let mut doc = one_page( + b"/OC /M BDC BT /F1 10 Tf 10 100 Td (VISIBLE) Tj ET EMC", + "/Properties << /M 9 0 R >>", + vec![ + b"<< /Type /OCG /Name (On) >>".to_vec(), + b"<< /Type /OCG /Name (Off) >>".to_vec(), + b"<< /Type /OCMD /OCGs [7 0 R 8 0 R] /P 12 0 R >>".to_vec(), + ], + ); + doc.set(ObjRef::new(12, 0), Object::name("AnyOn")); + let root = doc.root().unwrap(); + doc.update_dict(root, |d| { + d.set( + b"OCProperties".to_vec(), + Object::Dict(Dict::from_iter([ + (b"OCGs".to_vec(), Object::Array(vec![Object::Ref(ObjRef::new(7, 0)), Object::Ref(ObjRef::new(8, 0))])), + (b"D".to_vec(), Object::Dict(Dict::from_iter([(b"OFF".to_vec(), Object::Array(vec![Object::Ref(ObjRef::new(8, 0))]))]))), + ])), + ); + }) + .unwrap(); + + remove_hidden(&mut doc, &[Hidden::HiddenLayers]).unwrap(); + let saved = Document::open(Arc::new(write_full(&doc, &SaveOptions::default()).unwrap())).unwrap(); + assert!(pdfcraft_edit::text_lines(&saved, 0).unwrap().iter().any(|line| line.text.contains("VISIBLE"))); +} + +#[test] +fn surviving_anyoff_and_not_content_keeps_its_off_group_state() { + use crate::sanitize::{Hidden, remove_hidden}; + + for use_not in [false, true] { + let mut doc = one_page( + b"/OC /M BDC BT /F1 10 Tf 10 100 Td (VISIBLE) Tj ET EMC", + "/Properties << /M 9 0 R >>", + vec![ + b"<< /Type /OCG /Name (On) >>".to_vec(), + b"<< /Type /OCG /Name (Off) >>".to_vec(), + if use_not { b"<< /Type /OCMD /VE [/Not 8 0 R] >>".to_vec() } else { b"<< /Type /OCMD /OCGs [8 0 R] /P /AnyOff >>".to_vec() }, + ], + ); + let root = doc.root().unwrap(); + doc.update_dict(root, |d| { + d.set( + b"OCProperties".to_vec(), + Object::Dict(Dict::from_iter([ + (b"OCGs".to_vec(), Object::Array(vec![Object::Ref(ObjRef::new(7, 0)), Object::Ref(ObjRef::new(8, 0))])), + (b"D".to_vec(), Object::Dict(Dict::from_iter([(b"OFF".to_vec(), Object::Array(vec![Object::Ref(ObjRef::new(8, 0))]))]))), + ])), + ); + }) + .unwrap(); + + remove_hidden(&mut doc, &[Hidden::HiddenLayers]).unwrap(); + let root = doc.root().unwrap(); + let catalog = doc.get(root); + let oc = catalog.as_dict().and_then(|d| d.get(b"OCProperties")).map(|o| doc.resolve(o)); + let default = oc.as_deref().and_then(|o| o.as_dict()).and_then(|d| d.get(b"D")).map(|o| doc.resolve(o)); + let off = default.as_deref().and_then(|o| o.as_dict()).and_then(|d| d.get(b"OFF")).map(|o| doc.resolve(o)); + assert!( + off.as_deref().and_then(Object::as_array).is_some_and(|a| a.iter().any(|o| o.as_ref() == Some(ObjRef::new(8, 0)))), + "off group lost for /VE Not={use_not}" + ); + let saved = Document::open(Arc::new(write_full(&doc, &SaveOptions::default()).unwrap())).unwrap(); + assert!(pdfcraft_edit::text_lines(&saved, 0).unwrap().iter().any(|line| line.text.contains("VISIBLE"))); + } +} + #[test] fn hidden_ocmd_policy_matches_the_default_configuration() { use crate::sanitize::{Hidden, remove_hidden}; @@ -582,6 +692,10 @@ fn hidden_ocmd_policy_matches_the_default_configuration() { (Some("UnknownPolicy"), false, None, true), (None, true, None, true), (None, false, Some("and"), true), + (None, false, Some("and_single_not"), true), + (None, false, Some("and_single_off"), true), + (None, false, Some("or_single_on"), false), + (None, false, Some("or_single_off"), true), (Some("AllOn"), false, Some("not_off"), false), (None, false, Some("malformed_not"), true), (None, false, Some("malformed_without_ocgs"), true), @@ -609,6 +723,13 @@ fn hidden_ocmd_policy_matches_the_default_configuration() { match visibility { Some("and") => membership .set(b"VE".to_vec(), Object::Array(vec![Object::name("And"), Object::Ref(ObjRef::new(7, 0)), Object::Ref(ObjRef::new(8, 0))])), + Some("and_single_not") => membership.set( + b"VE".to_vec(), + Object::Array(vec![Object::name("Not"), Object::Array(vec![Object::name("And"), Object::Ref(ObjRef::new(7, 0))])]), + ), + Some("and_single_off") => membership.set(b"VE".to_vec(), Object::Array(vec![Object::name("And"), Object::Ref(ObjRef::new(8, 0))])), + Some("or_single_on") => membership.set(b"VE".to_vec(), Object::Array(vec![Object::name("Or"), Object::Ref(ObjRef::new(7, 0))])), + Some("or_single_off") => membership.set(b"VE".to_vec(), Object::Array(vec![Object::name("Or"), Object::Ref(ObjRef::new(8, 0))])), Some("not_off") => membership.set(b"VE".to_vec(), Object::Array(vec![Object::name("Not"), Object::Ref(ObjRef::new(8, 0))])), Some("malformed_not") => membership .set(b"VE".to_vec(), Object::Array(vec![Object::name("Not"), Object::Ref(ObjRef::new(7, 0)), Object::Ref(ObjRef::new(8, 0))])),