From dd20be5f91177fccc9fc2cd69450ffb01d67e7fc Mon Sep 17 00:00:00 2001 From: skylar taylor-barrick Date: Mon, 19 Oct 2026 12:24:55 -0600 Subject: [PATCH] fix(redact): evaluate OCMD visibility during sanitization Fixes #3499 by evaluating OCMD policies and bounded visibility expressions against the default optional-content configuration before removing hidden groups. Unknown or malformed visibility data is handled conservatively. --- crates/redact/src/interp.rs | 111 +++++++++++++++++++++++++- crates/redact/src/tests.rs | 151 ++++++++++++++++++++++++++++++++++++ 2 files changed, 260 insertions(+), 2 deletions(-) diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index 52ca6c276..7417f05a0 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -124,7 +124,8 @@ impl<'a> Scope<'a> { { return true; } - // A membership dictionary (default policy AnyOn): hidden when all its groups are. + // Evaluate the policy against the same default configuration used to select hidden + // groups. A membership dictionary's default policy is AnyOn. let d = doc.resolve(o); let Some(d) = d.as_dict() else { return false }; if d.name(b"Type") != Some(b"OCMD") { @@ -135,7 +136,113 @@ impl<'a> Scope<'a> { Some(_) => d.get(b"OCGs").and_then(Object::as_ref).into_iter().collect(), None => Vec::new(), }; - !groups.is_empty() && groups.iter().all(|g| self.hidden_layers.contains(g)) + let fail_closed = || groups.iter().any(|g| self.hidden_layers.contains(g)); + 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)); + } + if groups.is_empty() { + 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(), + } + } + + /// Evaluate an optional-content visibility expression (`/VE`) against the default config. + /// Malformed, cyclic, or excessive expressions return `None` so the caller can fail closed. + fn visibility_expression( + &self, + doc: &Document, + expression: &Object, + depth: usize, + path: &mut HashSet, + budget: &mut usize, + ) -> Option { + if depth > 64 || *budget == 0 { + return None; + } + *budget -= 1; + let reference = expression.as_ref(); + if let Some(r) = reference + && !path.insert(r) + { + return None; + } + let resolved = doc.resolve(expression); + let result = match &*resolved { + Object::Dict(d) if d.name(b"Type") == Some(b"OCG") => reference.map(|r| !self.hidden_layers.contains(&r)), + Object::Array(items) if (2..=4097).contains(&items.len()) => { + let operator = match items.first() { + Some(Object::Name(n)) => n.as_slice(), + _ => return None, + }; + match operator { + b"And" if items.len() >= 3 => { + 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 => { + let mut visible = false; + for item in items.iter().skip(1) { + visible |= self.visibility_expression(doc, item, depth + 1, path, budget)?; + } + Some(visible) + } + b"Not" if items.len() == 2 => Some(!self.visibility_expression(doc, &items[1], depth + 1, path, budget)?), + _ => None, + } + } + _ => None, + }; + if let Some(r) = reference { + path.remove(&r); + } + result + } + + /// Conservatively find hidden groups in an expression that could not be evaluated. Unknown + /// syntax, cycles, and exhausted budgets count as a possible hidden reference. + fn expression_mentions_hidden(&self, doc: &Document, expression: &Object, depth: usize, path: &mut HashSet, budget: &mut usize) -> bool { + if depth > 64 || *budget == 0 { + return true; + } + *budget -= 1; + if let Some(r) = expression.as_ref() { + if self.hidden_layers.contains(&r) { + return true; + } + if !path.insert(r) { + return true; + } + } + let resolved = doc.resolve(expression); + let found = match &*resolved { + Object::Array(items) => items.iter().any(|item| self.expression_mentions_hidden(doc, item, depth + 1, path, budget)), + Object::Dict(d) if d.name(b"Type") == Some(b"OCG") => expression.as_ref().is_none_or(|r| self.hidden_layers.contains(&r)), + Object::Dict(_) => true, + _ => false, + }; + if let Some(r) = expression.as_ref() { + path.remove(&r); + } + found } fn hits(&self, b: [f64; 4]) -> bool { diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index da1ecf12a..d6931116c 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -492,6 +492,157 @@ fn a_hidden_layers_image_does_not_survive_through_inherited_resources() { assert!(!bytes.windows(16).any(|w| w == b"HIDDENLAYERIMAGE"), "the off layer's image survived the save"); } +#[test] +fn hidden_ocmd_all_on_content_is_removed_before_its_off_group_is_unlinked() { + use crate::sanitize::{Hidden, remove_hidden}; + + let mut doc = one_page( + b"BT /F1 10 Tf 10 200 Td (PUBLIC) Tj ET /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 (Off) >>".to_vec(), + b"<< /Type /OCMD /OCGs [7 0 R 8 0 R] /P /AllOn >>".to_vec(), + ], + ); + let root = doc.root().unwrap(); + doc.update_dict(root, |d| { + d.set( + b"OCProperties".to_vec(), + Object::Dict(pdfcraft_cos::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(pdfcraft_cos::Dict::from_iter([ + (b"Order".to_vec(), Object::Array(vec![Object::Ref(ObjRef::new(7, 0)), Object::Ref(ObjRef::new(8, 0))])), + (b"OFF".to_vec(), Object::Array(vec![Object::Ref(ObjRef::new(8, 0))])), + ])), + ), + ])), + ); + }) + .unwrap(); + + let before = pdfcraft_edit::text_lines(&doc, 0).unwrap(); + assert!(before.iter().any(|line| line.text.contains("PUBLIC"))); + assert!(before.iter().any(|line| line.text.contains("SECRET")), "extractor sees source text before sanitization"); + remove_hidden(&mut doc, &[Hidden::HiddenLayers]).unwrap(); + + let saved = Document::open(Arc::new(write_full(&doc, &SaveOptions::default()).unwrap())).unwrap(); + let lines = pdfcraft_edit::text_lines(&saved, 0).unwrap(); + assert!(lines.iter().any(|line| line.text.contains("PUBLIC")), "visible content remains: {lines:?}"); + assert!(!lines.iter().any(|line| line.text.contains("SECRET")), "hidden text is extractable: {lines:?}"); + assert!(!write_full(&saved, &SaveOptions::default()).unwrap().windows(6).any(|w| w == b"SECRET")); +} + +#[test] +fn hidden_ocmd_content_in_a_form_and_nested_marked_blocks_is_removed() { + use crate::sanitize::{Hidden, remove_hidden}; + + let mut doc = one_page( + b"/Fm Do", + "/Properties << /M 9 0 R >> /XObject << /Fm 10 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 /AllOn >>".to_vec(), + stream( + "/Type /XObject /Subtype /Form /BBox [0 0 300 300] /Resources << /Font << /F1 5 0 R >> /Properties << /M 9 0 R >> >>", + b"/OC /M BDC /OC /M BDC BT /F1 10 Tf 10 100 Td (SECRET) Tj ET EMC EMC", + ), + ], + ); + 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("SECRET"))); +} + +#[test] +fn hidden_ocmd_policy_matches_the_default_configuration() { + use crate::sanitize::{Hidden, remove_hidden}; + + let cases = [ + (None, false, None, false), + (Some("AnyOn"), false, None, false), + (Some("AllOn"), false, None, true), + (Some("AnyOff"), false, None, false), + (Some("AllOff"), false, None, true), + (Some("UnknownPolicy"), false, None, true), + (None, true, None, true), + (None, false, Some("and"), true), + (Some("AllOn"), false, Some("not_off"), false), + (None, false, Some("malformed_not"), true), + (None, false, Some("malformed_without_ocgs"), true), + ]; + for (policy, malformed_policy, visibility, should_remove) in cases { + 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 (Off) >>".to_vec(), + b"<< /Type /OCMD /OCGs [7 0 R 8 0 R] /P /AllOn >>".to_vec(), + ], + ); + let mut membership = Dict::new(); + membership.set(b"Type".to_vec(), Object::name("OCMD")); + if visibility != Some("malformed_without_ocgs") { + membership.set(b"OCGs".to_vec(), Object::Array(vec![Object::Ref(ObjRef::new(7, 0)), Object::Ref(ObjRef::new(8, 0))])); + } + if let Some(policy) = policy { + membership.set(b"P".to_vec(), Object::name(policy)); + } else if malformed_policy { + membership.set(b"P".to_vec(), Object::Int(17)); + } + 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("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))])), + Some("malformed_without_ocgs") => { + membership.set(b"VE".to_vec(), Object::Array(vec![Object::name("UnknownOperator"), Object::Ref(ObjRef::new(8, 0))])) + } + _ => {} + } + doc.set(ObjRef::new(9, 0), Object::Dict(membership)); + 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"Order".to_vec(), Object::Array(vec![Object::Ref(ObjRef::new(7, 0)), Object::Ref(ObjRef::new(8, 0))])), + (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(); + let has_secret = pdfcraft_edit::text_lines(&saved, 0).unwrap().iter().any(|line| line.text.contains("SECRET")); + assert_eq!(has_secret, !should_remove, "policy {policy:?}, malformed={malformed_policy}, VE={visibility:?}"); + } +} + #[test] fn redacting_an_xfa_field_drops_the_xfa_packet_so_its_value_cannot_survive() { // A static AcroForm+XFA form whose field value lives both in the widget /V and in the XFA