diff --git a/crates/create/src/lib.rs b/crates/create/src/lib.rs index 10887a4be..aa6e8f6f2 100644 --- a/crates/create/src/lib.rs +++ b/crates/create/src/lib.rs @@ -314,10 +314,13 @@ fn png_image(name: &str, bytes: &[u8]) -> Result { _ => (72.0, 72.0), }; let icc = reader.info().icc_profile.as_ref().map(|p| p.to_vec()); - // The png crate's limits don't cover the frame buffer, sized by the header alone: a small - // file claiming enormous dimensions must not make it allocate (and fill) gigabytes. + // The decoded size comes from the header alone, so a file of a few bytes can claim any + // size: keep the ceiling, and ask for the memory fallibly (a failed allocation would abort + // the whole app, and no `catch_unwind` stops that). let size = reader.output_buffer_size().filter(|n| *n <= MAX_DECODED_BYTES).ok_or_else(|| bad("image too large".into()))?; - let mut buf = vec![0; size]; + let mut buf = Vec::new(); + buf.try_reserve_exact(size).map_err(|_| bad("not enough memory for this image".into()))?; + buf.resize(size, 0); let frame = reader.next_frame(&mut buf).map_err(|e| bad(e.to_string()))?; let (w, h) = (frame.width, frame.height); let data = &buf[..frame.buffer_size()]; @@ -330,8 +333,14 @@ fn png_image(name: &str, bytes: &[u8]) -> Result { }; let has_alpha = channels == 2 || channels == 4; let colour_n = if has_alpha { channels - 1 } else { channels }; - let mut colour = Vec::with_capacity(w as usize * h as usize * colour_n); + let mut colour = Vec::new(); + colour + .try_reserve_exact((w as usize).saturating_mul(h as usize).saturating_mul(colour_n)) + .map_err(|_| bad("not enough memory for this image".into()))?; let mut alpha = Vec::new(); + if has_alpha { + alpha.try_reserve_exact((w as usize).saturating_mul(h as usize)).map_err(|_| bad("not enough memory for this image".into()))?; + } for px in data.chunks_exact(channels) { colour.extend_from_slice(&px[..colour_n]); if has_alpha { diff --git a/crates/create/src/tests.rs b/crates/create/src/tests.rs index 45d376220..79732121d 100644 --- a/crates/create/src/tests.rs +++ b/crates/create/src/tests.rs @@ -198,6 +198,45 @@ fn a_tiff_claiming_enormous_dimensions_is_refused() { assert!(matches!(from_images(&[("huge.tif".into(), tif)]), Err(CreateError::Image(..)))); } +/// A PNG of a few bytes whose header claims `w`×`h` 8-bit RGBA pixels, with no pixel data. +fn png_claiming(w: u32, h: u32) -> Vec { + fn crc32(bytes: &[u8]) -> u32 { + let mut c = !0u32; + for &b in bytes { + c ^= u32::from(b); + for _ in 0..8 { + c = if c & 1 != 0 { 0xEDB8_8320 ^ (c >> 1) } else { c >> 1 }; + } + } + !c + } + let mut png = b"\x89PNG\r\n\x1a\n".to_vec(); + let mut chunk = |kind: &[u8], data: &[u8]| { + png.extend_from_slice(&(data.len() as u32).to_be_bytes()); + let body = [kind, data].concat(); + png.extend_from_slice(&body); + png.extend_from_slice(&crc32(&body).to_be_bytes()); + }; + let ihdr = [&w.to_be_bytes()[..], &h.to_be_bytes(), &[8, 6, 0, 0, 0]].concat(); + chunk(b"IHDR", &ihdr); + // An empty zlib stream: one stored block of no bytes. + chunk(b"IDAT", &[0x78, 0x01, 0x01, 0x00, 0x00, 0xff, 0xff, 0x00, 0x00, 0x00, 0x01]); + chunk(b"IEND", &[]); + png +} + +#[test] +fn a_tiny_png_claiming_huge_dimensions_is_refused_before_allocating() { + // 68 bytes claiming 13 GiB (and 1 TiB) of pixels: refused before anything is allocated, + // where it used to abort the process when the allocation failed. + for (w, h) in [(60_000, 60_000), (1 << 20, 1 << 18)] { + let png = png_claiming(w, h); + assert_eq!(png.len(), 68); + let r = from_images(&[("huge.png".into(), png)]); + assert!(matches!(&r, Err(CreateError::Image(_, m)) if m.contains("too large")), "{w}x{h}"); + } +} + /// Likewise a PNG: the png crate's limits don't cover the frame buffer, which its header sizes /// (60000x60000 RGBA is 14.4 GB). Refused before anything is allocated, not after. #[test] diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index 4a607dddf..cd496c33a 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -38,6 +38,10 @@ pub(crate) enum Mode { struct Gs { ctm: Matrix, font: Rc, + /// The current font writes vertically (a Type0 font with an `-V` encoding or a CMap whose + /// `/WMode` is 1). Glyph layout here is horizontal, so such text cannot be placed under a + /// region; apply fails closed when it meets any (see `show`). + vertical: bool, size: f64, char_spacing: f64, word_spacing: f64, @@ -54,8 +58,15 @@ pub(crate) struct Output { pub streams: Vec>>, /// New XObjects the rewritten content refers to: (resource name, object). pub xobjects: Vec<(Vec, ObjRef)>, + /// Resource names whose `Do` this scope dropped or pointed at a cleaned copy, so the caller + /// can unlink the original from the resource dictionary (a full save keeps whatever is still + /// referenced, which would otherwise retain the removed image or form bytes). + pub replaced: Vec>, /// Glyphs / inline images still overlapping a region (verify mode). pub residue: usize, + /// Apply mode met vertical-writing text (which it cannot remove) while a redaction region was + /// in force: the operation must fail closed rather than leave that text under the mark. + pub vertical: bool, } pub(crate) struct Scope<'a> { @@ -148,6 +159,20 @@ fn res_dict(doc: &Document, res: &Dict, key: &[u8]) -> Dict { res.get(key).map(|o| doc.resolve(o)).and_then(|o| o.as_dict().cloned()).unwrap_or_default() } +/// Does this font write vertically? Only a Type0 (composite) font can: its `/Encoding` is a +/// predefined CMap name ending in `-V` (Identity-V and the like), or an embedded CMap stream +/// whose `/WMode` is 1 (ISO 32000-2 §9.7.5). +fn is_vertical(doc: &Document, font: &Dict) -> bool { + if font.name(b"Subtype") != Some(b"Type0".as_slice()) { + return false; + } + match font.get(b"Encoding").map(|e| doc.resolve(e)).as_deref() { + Some(Object::Name(n)) => n.ends_with(b"-V"), + Some(Object::Stream(s)) => s.dict.int(b"WMode") == Some(1), + _ => false, + } +} + fn fresh_name(scope: &mut Scope<'_>, xobjects: &Dict) -> Vec { let mut i = scope.used_names.len() + 1; loop { @@ -160,6 +185,23 @@ fn fresh_name(scope: &mut Scope<'_>, xobjects: &Dict) -> Vec { } } +/// Drop every `replaced` resource name whose `Do` no longer appears in the rewritten `content`, +/// so a full-rewrite save stops reaching the original image or form it named. A name that some +/// surviving `Do` still draws (the same image shown again outside a mark) is kept. +pub(crate) fn prune(xo: &mut Dict, replaced: &[Vec], content: &[Vec]) { + if replaced.is_empty() { + return; + } + let joined = Pieces::join(content); + let ops = joined.parse().ops; + let still_drawn = |n: &[u8]| ops.iter().any(|op| op.is("Do") && op.name(0) == Some(n)); + for n in replaced { + if !still_drawn(n) { + xo.remove(n); + } + } +} + /// The clip that hides everything inside each region: one even-odd clip per region (a big /// rectangle with the region cut out), in the current user space. Returns `None` when the CTM /// can't be inverted (nothing is painted then anyway). @@ -195,8 +237,18 @@ struct Glyph { pub(crate) fn process(doc: &mut Document, scope: &mut Scope<'_>, streams: &[Vec], resources: &Dict, ctm: Matrix) -> Output { let mut out = Output::default(); let fallback = Rc::new(Metrics::fallback()); - let mut gs = - Gs { ctm, font: fallback.clone(), size: 0.0, char_spacing: 0.0, word_spacing: 0.0, render_mode: 0, scale: 1.0, leading: 0.0, rise: 0.0 }; + let mut gs = Gs { + ctm, + font: fallback.clone(), + vertical: false, + size: 0.0, + char_spacing: 0.0, + word_spacing: 0.0, + render_mode: 0, + scale: 1.0, + leading: 0.0, + rise: 0.0, + }; let mut stack: Vec = Vec::new(); let (mut tm, mut tlm) = (Matrix::IDENTITY, Matrix::IDENTITY); let fonts_res = res_dict(doc, resources, b"Font"); @@ -329,14 +381,12 @@ pub(crate) fn process(doc: &mut Document, scope: &mut Scope<'_>, streams: &[Vec< b"Tf" => { gs.size = op.num(1).unwrap_or(gs.size); if let Some(name) = op.name(0) { + let font_dict = fonts_res.get(name).and_then(|f| doc.resolve(f).as_dict().cloned()); + gs.vertical = font_dict.as_ref().is_some_and(|d| is_vertical(doc, d)); gs.font = match scope.fonts.get(name) { Some(f) => f.clone(), None => { - let m = fonts_res - .get(name) - .and_then(|f| doc.resolve(f).as_dict().cloned()) - .map(|d| Rc::new(Metrics::from_dict(doc, &d))) - .unwrap_or_else(|| fallback.clone()); + let m = font_dict.as_ref().map(|d| Rc::new(Metrics::from_dict(doc, d))).unwrap_or_else(|| fallback.clone()); scope.fonts.insert(name.to_vec(), m.clone()); m } @@ -381,6 +431,18 @@ pub(crate) fn process(doc: &mut Document, scope: &mut Scope<'_>, streams: &[Vec< b"TJ" => op.operands.first().and_then(Object::as_array).cloned().unwrap_or_default(), _ => op.operands.last().cloned().into_iter().collect(), }; + // Vertical-writing text is laid out horizontally by `show`, so its glyphs are + // never matched against a region. Rather than ship a document whose vertical + // text was silently left under a mark, record it so apply can fail closed. + // (Full vertical-text removal is a future enhancement.) + if gs.vertical + && scope.mode == Mode::Apply + && scope.hidden_text.is_none() + && !scope.rects.is_empty() + && items.iter().any(|o| matches!(o, Object::String(s) if !s.bytes.is_empty())) + { + out.vertical = true; + } let (new_items, removed) = show(scope, &gs, &mut tm, &items); if removed == 0 { ops.push(op); @@ -543,6 +605,7 @@ fn xobject( return None; } scope.layer_blocks += 1; + out.replaced.push(name); Some(None) } Some(b"Image") => { @@ -552,6 +615,7 @@ fn xobject( } if scope.covered(b) { scope.report.images_removed += 1; + out.replaced.push(name); return Some(None); } match image::clear(doc, s, &gs.ctm, scope.rects) { @@ -561,11 +625,13 @@ fn xobject( let n = fresh_name(scope, xobjects); xobjects.set(n.clone(), Object::Ref(nr)); out.xobjects.push((n.clone(), nr)); + out.replaced.push(name); scope.report.images_cleared += 1; Some(Some(Op::new("Do", vec![Object::Name(n)]))) } Err(()) => { scope.report.images_removed += 1; + out.replaced.push(name); Some(None) } } @@ -588,12 +654,14 @@ fn xobject( return None; } scope.report.forms_removed += 1; + out.replaced.push(name); return Some(None); } let data = match s.decoded() { Ok(d) => d, Err(_) if scope.mode == Mode::Apply => { scope.report.forms_removed += 1; + out.replaced.push(name); return Some(None); } Err(_) => return None, @@ -606,18 +674,24 @@ fn xobject( scope.depth -= 1; scope.fonts = saved_fonts; out.residue += inner.residue; + out.vertical |= inner.vertical; if scope.mode == Mode::Verify { return None; } let new_data = inner.streams.into_iter().next().flatten(); - if new_data.is_none() && inner.xobjects.is_empty() { + if new_data.is_none() && inner.xobjects.is_empty() && inner.replaced.is_empty() { return None; } let mut dict = s.dict.clone(); dict.remove(b"Length"); - if !inner.xobjects.is_empty() { + // Rebuild the copy's /Resources whenever inner content gained or lost an XObject, so a + // `Do` the copy no longer makes stops keeping the original image or form reachable. + if !inner.xobjects.is_empty() || !inner.replaced.is_empty() { let mut res = res; let mut xo = res_dict(doc, &res, b"XObject"); + if let Some(d) = &new_data { + prune(&mut xo, &inner.replaced, std::slice::from_ref(d)); + } for (n, r) in &inner.xobjects { xo.set(n.clone(), Object::Ref(*r)); } @@ -632,6 +706,7 @@ fn xobject( let n = fresh_name(scope, xobjects); xobjects.set(n.clone(), Object::Ref(nr)); out.xobjects.push((n.clone(), nr)); + out.replaced.push(name); scope.report.forms_rewritten += 1; Some(Some(Op::new("Do", vec![Object::Name(n)]))) } diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index 17db7404d..4ceef50bf 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -37,6 +37,8 @@ pub enum RedactError { Unreadable(usize), #[error("redaction could not be verified: {0} item(s) were still found under the marks, so nothing was changed")] Residue(usize), + #[error("page {0} has vertical-writing text under a redaction mark, which PdfCraft cannot remove yet, so nothing was changed")] + VerticalText(usize), #[error(transparent)] Edit(#[from] pdfcraft_edit::EditError), #[error(transparent)] @@ -107,6 +109,9 @@ pub struct Report { pub fields: usize, /// Structure elements that lost alternate/actual text or emptied marked content. pub tags: usize, + /// The form was an AcroForm+XFA form and its `/XFA` packets (which still held redacted field + /// values, and from which a relayout could rebuild them) were removed. + pub xfa_removed: bool, } fn rect_of(doc: &Document, o: Option<&Object>) -> Option<[f64; 4]> { @@ -292,6 +297,13 @@ pub fn apply(doc: &mut Document, pages: Option<&[usize]>) -> Result) -> Result> = new_list + .iter() + .map(|o| match &*doc.resolve(o) { + Object::Stream(s) => s.decoded().unwrap_or_default(), + _ => Vec::new(), + }) + .collect(); + interp::prune(&mut xo, &out.replaced, &page_content); for (n, r) in &out.xobjects { xo.set(n.clone(), Object::Ref(*r)); } @@ -388,6 +410,37 @@ pub fn apply(doc: &mut Document, pages: Option<&[usize]>) -> Result { + doc.update_dict(af, strip_xfa)?; + doc.update_dict(root, |d| { + d.remove(b"NeedsRendering"); + })?; + report.xfa_removed = true; + } + Some(Object::Dict(mut af)) if af.contains(b"XFA") => { + strip_xfa(&mut af); + doc.update_dict(root, |d| { + d.set(b"AcroForm".to_vec(), Object::Dict(af)); + d.remove(b"NeedsRendering"); + })?; + report.xfa_removed = true; + } + _ => {} + } + } + // 4. The boxes. for &pi in &by_page { let page_marks: Vec<&Mark> = chosen.iter().copied().filter(|m| m.page == pi).collect(); diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index 11756bba1..180e6cc94 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -178,10 +178,20 @@ fn content_pass(doc: &mut Document, text: bool, layers: bool, write: bool) -> Re new_list[i] = Object::Ref(doc.add(Object::Stream(pdfcraft_cos::Stream::flate(dict, &bytes)))); changed = true; } - if changed || !out.xobjects.is_empty() { + if changed || !out.xobjects.is_empty() || !out.replaced.is_empty() { let mut res = resources; - if !out.xobjects.is_empty() { + if !out.xobjects.is_empty() || !out.replaced.is_empty() { let mut xo = res.get(b"XObject").and_then(|x| doc.resolve(x).as_dict().cloned()).unwrap_or_default(); + // A full save keeps whatever is still referenced: unlink the originals this page no + // longer draws so the removed form or image bytes cannot survive in the saved file. + let page_content: Vec> = new_list + .iter() + .map(|o| match &*doc.resolve(o) { + Object::Stream(s) => s.decoded().unwrap_or_default(), + _ => Vec::new(), + }) + .collect(); + crate::interp::prune(&mut xo, &out.replaced, &page_content); for (n, r) in &out.xobjects { xo.set(n.clone(), Object::Ref(*r)); } diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index c8b913c00..ef725674b 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -203,6 +203,48 @@ fn rotated_text_and_composite_fonts() { assert!(c.contains("[<0001> -750 <0004>] TJ") || c.contains("[(\\000\\001) -750 (\\000\\004)] TJ"), "{c}"); } +#[test] +fn vertical_writing_text_under_a_mark_fails_closed() { + // A Type0 font laid out vertically (Identity-V / CMap WMode 1) cannot have its glyphs placed + // under a mark by the horizontal interpreter, so Apply Redactions must fail rather than ship + // the text under the box. The horizontal control (Identity-H) is redacted normally. + let make = |encoding: &str| -> Document { + let t0 = format!( + "<< /Type /Font /Subtype /Type0 /BaseFont /X /Encoding /{encoding} /DescendantFonts [<< /Type /Font /Subtype /CIDFontType2 /BaseFont /X /DW 1000 /W [1 [500 500] 3 4 250] >>] >>" + ) + .into_bytes(); + let src = b"BT /F2 10 Tf 0 1 -1 0 100 50 Tm <0001000200030004> Tj ET"; + let mut doc = one_page(src, "", vec![]); + let font_ref = doc.add(Object::Dict(Dict::new())); + let parsed = { + let mut lx = pdfcraft_cos::Lexer::new(&t0, 0); + lx.object().unwrap() + }; + doc.set(font_ref, parsed); + let page = pdfcraft_model::pages(&doc)[0].obj; + doc.update_dict(page, |d| { + let mut res = d.get(b"Resources").and_then(Object::as_dict).cloned().unwrap(); + let mut fonts = res.get(b"Font").and_then(Object::as_dict).cloned().unwrap(); + fonts.set(b"F2".to_vec(), Object::Ref(font_ref)); + res.set(b"Font".to_vec(), Object::Dict(fonts)); + d.set(b"Resources".to_vec(), Object::Dict(res)); + }) + .unwrap(); + doc + }; + + // Identity-V: apply must refuse rather than report success with the text still in the file. + let mut vdoc = make("Identity-V"); + mark(&mut vdoc, 0, &[[85.0, 55.5, 105.0, 61.0]], ""); + assert_eq!(apply(&mut vdoc, None), Err(RedactError::VerticalText(1)), "vertical text under a mark must not silently pass"); + + // Identity-H control: the same glyphs and mark are redacted normally. + let mut hdoc = make("Identity-H"); + mark(&mut hdoc, 0, &[[85.0, 55.5, 105.0, 61.0]], ""); + let r = apply(&mut hdoc, None).unwrap(); + assert_eq!(r.glyphs, 2, "the horizontal control is redacted: {r:?}"); +} + #[test] fn images_are_removed_or_have_their_pixels_cleared() { // Image 1 is fully covered; image 2 (4×1 gray, all white) is half covered. @@ -262,6 +304,94 @@ fn shared_form_xobjects_are_copied_not_changed() { assert_eq!(under(&mut doc, 0, &[[0.0, 90.0, 300.0, 120.0]]), 0); } +#[test] +fn redacted_images_and_forms_do_not_survive_a_full_rewrite_save() { + // Page 1 draws a fully covered image, a partly covered image and a form XObject whose text + // is under the mark. After Apply Redactions and the forced full rewrite, none of their secret + // bytes may stay in the saved file, while an unrelated image drawn only on page 2 survives. + let im1 = b"REDIMGONECOVERED"; // 16 bytes, one gray row + let impart = b"REDIMGPARTSECRETPUBPARTBOTTOMROW"; // 16-byte secret top row + 16-byte bottom row + let imkeep = b"PUBIMGKEEPSURVIVE"; // 17 bytes, used only on page 2 + let objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R 10 0 R] /Count 2 >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources << /Font << /F1 5 0 R >> /XObject << /Im1 7 0 R /ImPart 8 0 R /FmSecret 9 0 R >> >> >>".to_vec(), + stream("", b"q 50 0 0 50 20 230 cm /Im1 Do Q q 50 0 0 100 100 180 cm /ImPart Do Q /FmSecret Do"), + FONT.replace("95 0 R", "6 0 R").into_bytes(), + widths(), + stream("/Type /XObject /Subtype /Image /Width 16 /Height 1 /ColorSpace /DeviceGray /BitsPerComponent 8", im1), + stream("/Type /XObject /Subtype /Image /Width 16 /Height 2 /ColorSpace /DeviceGray /BitsPerComponent 8", impart), + stream( + "/Type /XObject /Subtype /Form /BBox [0 0 300 300] /Resources << /Font << /F1 5 0 R >> >>", + b"BT /F1 10 Tf 20 230 Td (REDFORMSECRETTEXT) Tj ET", + ), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 11 0 R /Resources << /XObject << /ImKeep 12 0 R >> >> >>".to_vec(), + stream("", b"q 50 0 0 50 20 20 cm /ImKeep Do Q"), + stream("/Type /XObject /Subtype /Image /Width 17 /Height 1 /ColorSpace /DeviceGray /BitsPerComponent 8", imkeep), + ]; + let mut doc = pdf(objs); + mark(&mut doc, 0, &[[0.0, 200.0, 300.0, 300.0]], ""); + let r = apply(&mut doc, None).unwrap(); + assert!(r.images_removed >= 1 && r.forms_rewritten >= 1, "{r:?}"); + + // The full rewrite that redaction forces must not carry the removed content forward. + let bytes = write_full(&doc, &SaveOptions::default()).unwrap(); + let has = |needle: &[u8]| bytes.windows(needle.len()).any(|w| w == needle); + assert!(!has(b"REDIMGONECOVERED"), "covered image samples survived the save"); + assert!(!has(b"REDIMGPARTSECRET"), "partly covered image's secret row survived the save"); + assert!(!has(b"REDFORMSECRETTEXT"), "the form's redacted text survived the save"); + + // The unrelated image drawn only on page 2 stays referenced and intact. + let saved = Document::open(Arc::new(bytes)).unwrap(); + let keep = pdfcraft_model::pages(&saved).swap_remove(1); + let res = saved.resolve(keep.dict.get(b"Resources").unwrap()).as_dict().cloned().unwrap(); + let xo = saved.resolve(res.get(b"XObject").unwrap()).as_dict().cloned().unwrap(); + let kref = xo.get(b"ImKeep").and_then(Object::as_ref).expect("ImKeep still referenced"); + let Object::Stream(s) = &*saved.get(kref) else { panic!("ImKeep is a stream") }; + assert_eq!(s.decoded().unwrap(), imkeep, "the unrelated image survives intact"); +} + +#[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 + // datasets packet (Designer-style /XFA array). Redacting the widget must delete the field and + // stop the form being XFA, so neither the saved file nor a later relayout keeps the value. + let objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 2 0 R /NeedsRendering true /AcroForm 7 0 R >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources << /Font << /F1 5 0 R >> >> /Annots [8 0 R] >>".to_vec(), + stream("", b"BT ET"), + FONT.replace("95 0 R", "6 0 R").into_bytes(), + widths(), + b"<< /Fields [8 0 R] /XFA 9 0 R /PCXfaLayout << /Pages 1 /Fields 1 >> >>".to_vec(), + b"<< /Type /Annot /Subtype /Widget /FT /Tx /T (ssn) /V (SSN-987-65-4321) /Rect [72 200 272 220] /P 3 0 R >>".to_vec(), + b"[(datasets) 10 0 R (template) 11 0 R]".to_vec(), + stream( + "", + b"SSN-987-65-4321", + ), + stream( + "", + b"", + ), + ]; + let mut doc = pdf(objs); + assert_eq!(pdfcraft_forms::fields(&doc).len(), 1, "the XFA-bound field is present before"); + mark(&mut doc, 0, &[[72.0, 200.0, 272.0, 220.0]], ""); + let r = apply(&mut doc, None).unwrap(); + assert!(r.fields >= 1 && r.xfa_removed, "{r:?}"); + + let bytes = write_full(&doc, &SaveOptions::default()).unwrap(); + assert!(!bytes.windows(15).any(|w| w == b"SSN-987-65-4321"), "the redacted value survived in the datasets packet"); + + let saved = Document::open(Arc::new(bytes)).unwrap(); + let cat = saved.get(saved.root().unwrap()).as_dict().cloned().unwrap(); + let acro = saved.resolve(cat.get(b"AcroForm").unwrap()).as_dict().cloned().unwrap(); + assert!(acro.get(b"XFA").is_none(), "/XFA is gone from the saved AcroForm"); + assert!(acro.get(b"PCXfaLayout").is_none(), "the cached layout mark is cleared"); + assert!(cat.get(b"NeedsRendering").is_none(), "/NeedsRendering is cleared"); +} + #[test] fn comments_links_and_fields_under_a_mark_go() { let mut doc = one_page(b"", "", vec![]); diff --git a/crates/sign/src/pdf.rs b/crates/sign/src/pdf.rs index 49bc9d4cf..e2eb6c617 100644 --- a/crates/sign/src/pdf.rs +++ b/crates/sign/src/pdf.rs @@ -976,6 +976,12 @@ fn classify_changes(doc: &Document, old: Option, p: Option) -> Mod .collect(); // The signed revision's document-level XMP stream. let old_metadata = old.root().and_then(|root| old.get(root).as_dict().and_then(|c| c.get(b"Metadata").and_then(Object::as_ref))); + // What each object was in the signed revision: its pages, and its trailer /Info. A changed + // object is judged by the role it held when signed, never by the labels or keys its new, + // attacker-written version carries (relabelling a signed page /Type /Sig, or dropping its + // /Type and adding /ModDate, must not reclassify it away from page content). + let signed_pages: HashSet = pdfcraft_annot::page_refs(&old).unwrap_or_default().into_iter().collect(); + let old_info = old.trailer().get(b"Info").and_then(Object::as_ref); let (mut allowed, mut disallowed): (Vec<&str>, Vec<&str>) = (Vec::new(), Vec::new()); let dss = dss_parts(doc, &old); // Stored at the same place in both (and not edited since): the same bytes, unchanged. @@ -1005,11 +1011,27 @@ fn classify_changes(doc: &Document, old: Option, p: Option) -> Mod let kind = match &*new { // The Document Security Store's own arrays and dictionaries, when they only grew. _ if dss.contains(&num) => "document security store", - Object::Dict(d) => change_kind(doc, &old, d, before.as_deref().and_then(Object::as_dict)), + Object::Dict(d) => { + let was = before.as_deref().and_then(Object::as_dict); + if signed_pages.contains(&r) { + // A signed page is always judged by what the page shows, whatever it is now + // relabelled: a rewritten /Contents or /Resources is a page content change. + page_change_kind(doc, &old, d, was) + } else if was.is_some() && Some(r) == old_info { + // Only the signed revision's own /Info dictionary is a metadata change. + "metadata" + } else { + change_kind(doc, &old, d, was) + } + } Object::Stream(s) => { + // File plumbing is what the stream was when signed: the /Type it has now is + // written by whoever changed it, and the renderer ignores /Type (a page's Form + // XObject relabelled /ObjStm still draws its new content). + let was_plumbing = matches!(before.as_deref(), Some(Object::Stream(b)) if matches!(b.dict.name(b"Type"), Some(b"XRef" | b"ObjStm"))); if old_content.contains(&r) { "page content" - } else if s.dict.name(b"Type") == Some(b"XRef") || s.dict.name(b"Type") == Some(b"ObjStm") || before.is_none() { + } else if before.is_none() || was_plumbing { continue; } else if s.dict.name(b"Type") == Some(b"Metadata") && Some(r) == old_metadata { // Only the signed catalog's XMP stream. A /Type /Metadata label on anything @@ -1210,15 +1232,42 @@ fn acroform_kind(doc: &Document, old: &Document, now: &Dict, before: Option<&Dic if added.is_empty() || added_annots(doc, &added) == "signature" { "signature" } else { "form fill" } } +/// Dictionary entries in a stable order, without `keys`. +fn without(x: &Dict, keys: &[&[u8]]) -> Vec<(Vec, Object)> { + let mut v: Vec<(Vec, Object)> = x.iter().filter(|(k, _)| !keys.contains(&k.as_slice())).map(|(k, o)| (k.to_vec(), o.clone())).collect(); + v.sort_by(|a, b| a.0.cmp(&b.0)); + v +} + +/// Whether a dictionary was a signature object: a signature or timestamp value, a signature +/// reference or its transform parameters, or a signature form field. Used to decide from the +/// signed revision, so a later relabel of some other object cannot pose as a signature change. +fn is_signature_dict(d: &Dict) -> bool { + matches!(d.name(b"Type"), Some(b"Sig" | b"DocTimeStamp" | b"SigRef" | b"TransformParams")) || d.name(b"FT") == Some(b"Sig") +} + +/// A changed object that was a page in the signed revision, judged by what the page shows rather +/// than by any `/Type` its new version carries. Only added annotations are permitted; any other +/// difference (content, resources, a relabelled or dropped `/Type`) is a page content change. +fn page_change_kind(doc: &Document, old: &Document, d: &Dict, before: Option<&Dict>) -> &'static str { + match before { + Some(b) if without(d, &[b"Annots"]) != without(b, &[b"Annots"]) => "page content", + Some(b) => { + let was = refs(old, b.get(b"Annots")); + let added: Vec = refs(doc, d.get(b"Annots")).into_iter().filter(|r| !was.contains(r)).collect(); + added_annots(doc, &added) + } + None => "page content", + } +} + fn change_kind(doc: &Document, old: &Document, d: &Dict, before: Option<&Dict>) -> &'static str { let ty = d.name(b"Type"); - let without = |x: &Dict, keys: &[&[u8]]| -> Vec<(Vec, Object)> { - let mut v: Vec<(Vec, Object)> = x.iter().filter(|(k, _)| !keys.contains(&k.as_slice())).map(|(k, o)| (k.to_vec(), o.clone())).collect(); - v.sort_by(|a, b| a.0.cmp(&b.0)); - v - }; + // A later revision cannot make an existing object a signature by relabelling it: a signature + // change is only a brand-new object, or one that was already a signature object when signed. + let signature_ok = before.is_none_or(is_signature_dict); match ty { - Some(b"Sig" | b"DocTimeStamp" | b"SigRef" | b"TransformParams") => return "signature", + Some(b"Sig" | b"DocTimeStamp" | b"SigRef" | b"TransformParams") if signature_ok => return "signature", Some(b"Catalog") => { let keys: &[&[u8]] = &[b"AcroForm", b"DSS", b"Perms", b"Metadata", b"NeedsRendering"]; return match before { @@ -1252,12 +1301,12 @@ fn change_kind(doc: &Document, old: &Document, d: &Dict, before: Option<&Dict>) _ => {} } match d.name(b"Subtype") { - Some(b"Widget") => return if d.name(b"FT") == Some(b"Sig") { "signature" } else { "form fill" }, + Some(b"Widget") => return if d.name(b"FT") == Some(b"Sig") && signature_ok { "signature" } else { "form fill" }, Some(b"Link") => return "links", Some(_) if ty == Some(b"Annot") || d.contains(b"Rect") => return "comments", _ => {} } - if d.name(b"FT") == Some(b"Sig") { + if d.name(b"FT") == Some(b"Sig") && signature_ok { return "signature"; } if d.contains(b"Fields") { @@ -1266,9 +1315,6 @@ fn change_kind(doc: &Document, old: &Document, d: &Dict, before: Option<&Dict>) if d.contains(b"FT") || (d.contains(b"T") && d.contains(b"Kids")) || d.contains(b"DR") { return "form fill"; } - if d.contains(b"Producer") || d.contains(b"ModDate") || d.contains(b"CreationDate") { - return "metadata"; - } if before.is_none() { // New objects referenced by changed ones (fonts, appearance resources). return "form fill"; diff --git a/crates/sign/tests/pdf.rs b/crates/sign/tests/pdf.rs index d2df432eb..80daee879 100644 --- a/crates/sign/tests/pdf.rs +++ b/crates/sign/tests/pdf.rs @@ -1116,6 +1116,104 @@ fn a_form_xobject_relabelled_metadata_is_not_a_metadata_change() { assert_eq!(s.modification, Modification::Allowed(vec!["metadata".into()])); } +#[test] +fn a_form_xobject_relabelled_as_file_plumbing_is_still_judged() { + // Whether a changed stream is an xref or object stream is what it was when signed: a + // drawn Form XObject rewritten after signing and labelled /ObjStm or /XRef still shows its + // new content (the renderer ignores /Type), so it must not be skipped. + let id = pkcs12::open(&data("rsa-aes.p12"), "test").unwrap(); + let trust = TrustStore { certs: vec![id.certificate.clone()] }; + for certify in LEVELS { + let signed = pdfcraft_sign::sign(&open(&fixture_with_xobject()), &id, &SignOptions { certify, ..opts() }).unwrap(); + for label in ["ObjStm", "XRef"] { + let edited = edit_after(&signed, |doc| { + let mut d = pdfcraft_cos::Dict::new(); + d.set(b"Type".to_vec(), Object::name(label)); + d.set(b"Subtype".to_vec(), Object::name("Form")); + d.set(b"BBox".to_vec(), Object::Array(vec![Object::Int(0), Object::Int(0), Object::Int(300), Object::Int(300)])); + let body = b"BT /F1 14 Tf 20 250 Td (Forged text) Tj ET".to_vec(); + d.set(b"Length".to_vec(), Object::Int(body.len() as i64)); + doc.set(pdfcraft_cos::ObjRef { num: 7, generation: 0 }, Object::Stream(pdfcraft_cos::Stream::from_raw(d, body))); + }); + let s = signatures(&open(&edited), &edited, &trust).into_iter().find(|s| s.signed).unwrap(); + assert_eq!(s.status, Status::Invalid, "{certify:?} /{label}: {:?}", s.details); + assert!(matches!(&s.modification, Modification::Disallowed(k) if k.iter().any(|k| k == "other changes")), "{:?}", s.modification); + } + } +} + +/// `fixture()` with a document information dictionary (object 7) named by the trailer's `/Info`. +fn fixture_with_info() -> Vec { + let objs: Vec<&[u8]> = vec![ + b"<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [6 0 R] >> >>", + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>", + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources << /Font << /F1 5 0 R >> >> /Annots [6 0 R] >>", + b"<< /Length 44 >>\nstream\nBT /F1 14 Tf 20 250 Td (Contract text) Tj ET\nendstream", + b"<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>", + b"<< /Type /Annot /Subtype /Widget /FT /Sig /T (Approval) /Rect [150 20 280 70] /P 3 0 R /F 4 >>", + b"<< /Producer (PdfCraft) /CreationDate (D:20260101000000Z) >>", + ]; + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n", i + 1).as_bytes()); + out.extend_from_slice(o); + out.extend_from_slice(b"\nendobj\n"); + } + let xref = out.len(); + out.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offsets { + out.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + out.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R /Info 7 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes()); + out +} + +#[test] +fn a_signed_page_relabelled_or_stripped_of_type_is_still_page_content() { + // A page is judged by the role it had when signed, not by the /Type its new version carries. + // A hand-written incremental revision that rewrites the signed page's /Contents while + // relabelling it /Type /Sig (permitted at every level) or dropping /Type and adding /ModDate + // (read as a metadata change) must still be a disallowed page content change at every level. + let forge_page = |ty: Option<&'static str>| { + move |doc: &mut Document| { + let forged = doc.add(stream_of(b"BT /F1 14 Tf 20 250 Td (Amount due: 999999 EUR) Tj ET")); + let mut entries = vec![ + ("Parent", oref(2)), + ("MediaBox", Object::Array(vec![Object::Int(0), Object::Int(0), Object::Int(300), Object::Int(300)])), + ("Contents", Object::Ref(forged)), + ("Resources", Object::Dict(dict_of(vec![("Font", Object::Dict(dict_of(vec![("F1", oref(5))])))]))), + ("Annots", Object::Array(vec![oref(6)])), + ]; + match ty { + Some(name) => entries.insert(0, ("Type", Object::name(name))), + None => entries.push(("ModDate", Object::String(PdfString::literal("D:20261003000000Z")))), + } + doc.set(pdfcraft_cos::ObjRef { num: 3, generation: 0 }, Object::Dict(dict_of(entries))); + } + }; + assert_refused_at_every_level(&fixture(), "page content", forge_page(Some("Sig"))); + assert_refused_at_every_level(&fixture(), "page content", forge_page(None)); + + // Positive control: an ordinary /Info /ModDate bump on the signed revision's own information + // dictionary stays a permitted metadata change. + let id = pkcs12::open(&data("rsa-aes.p12"), "test").unwrap(); + let trust = TrustStore { certs: vec![id.certificate.clone()] }; + for certify in LEVELS { + let signed = pdfcraft_sign::sign(&open(&fixture_with_info()), &id, &SignOptions { certify, ..opts() }).unwrap(); + let bumped = edit_after(&signed, |doc| { + doc.update_dict(pdfcraft_cos::ObjRef { num: 7, generation: 0 }, |d| { + d.set(b"ModDate".to_vec(), Object::String(PdfString::literal("D:20261003120000Z"))); + }) + .unwrap(); + }); + let s = signatures(&open(&bumped), &bumped, &trust).into_iter().find(|s| s.signed).unwrap(); + assert_eq!(s.status, Status::Valid, "{certify:?}: {:?}", s.details); + assert!(matches!(&s.modification, Modification::Allowed(k) if k.iter().any(|k| k == "metadata")), "{certify:?}: {:?}", s.modification); + } +} + /// A self-signed CRL from `id` revoking its own certificate, valid from `this` to `next`. fn self_crl(id: &pkcs12::DigitalId, this: Time, next: Time, revoked_at: Time) -> Vec { let alg = id.key.signature_algorithm(pdfcraft_sign::DigestAlg::Sha256); diff --git a/crates/ui-egui/src/sign_ui.rs b/crates/ui-egui/src/sign_ui.rs index 915f0c34b..2a759b1b4 100644 --- a/crates/ui-egui/src/sign_ui.rs +++ b/crates/ui-egui/src/sign_ui.rs @@ -1108,7 +1108,10 @@ pub(crate) fn panel(ui: &mut egui::Ui, t: &Tokens, sigs: &[SignatureInfo], expan } ui.add_space(4.0); ui.horizontal_wrapped(|ui| { - if let Some(c) = s.chain.last().or(s.certificate.as_ref()) + // Trust exactly the signer shown above (s.certificate), not the chain top the + // document embedded: trusting an issuer or root is a separate, explicit step in + // the Certificate Viewer's Trust tab, which names that certificate. + if let Some(c) = s.certificate.as_ref() && s.status == SignatureStatus::Unknown && ui.button(tl!("Add to trusted certificates")).clicked() { diff --git a/crates/ui-egui/tests/trust_signer.rs b/crates/ui-egui/tests/trust_signer.rs new file mode 100644 index 000000000..27bd2f3c9 --- /dev/null +++ b/crates/ui-egui/tests/trust_signer.rs @@ -0,0 +1,87 @@ +//! The Signatures panel's one-click "Add to trusted certificates" must store the signer shown in +//! the row, not the chain top the document embedded in the CMS (audit v3-098). Signing with a +//! certificate whose CMS carries its issuer chain gives a signature whose `chain.last()` (the +//! self-signed root) differs from its `certificate` (the signer); the click must trust the signer. + +use egui_kittest::Harness; +use egui_kittest::kittest::Queryable; +use pdfcraft_engine::SignatureStatus; +use pdfcraft_ui_egui::{PdfCraftApp, SignStep}; + +const FIXTURE: &[u8] = b"%PDF-1.7 +1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj +2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj +3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 300 200] /Contents 4 0 R /Resources << /Font << /F1 5 0 R >> >> >> endobj +4 0 obj << /Length 45 >> stream +BT /F1 14 Tf 20 150 Td (Please sign below) Tj ET +endstream endobj +5 0 obj << /Type /Font /Subtype /Type1 /BaseFont /Helvetica >> endobj +trailer << /Root 1 0 R >> +%%EOF"; + +fn dir() -> std::path::PathBuf { + static NEXT: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); + let n = NEXT.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let d = std::env::temp_dir().join(format!("pdfcraft-trust-signer-{}-{n}", std::process::id())); + let _ = std::fs::remove_dir_all(&d); + std::fs::create_dir_all(&d).unwrap(); + d +} + +fn harness(dir: std::path::PathBuf) -> Harness<'static, PdfCraftApp> { + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(move |_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("contract.pdf", None, FIXTURE.to_vec()).expect("opens"); + app.set_option("left", "closed").unwrap(); + app.export_dir_override = Some(dir.to_string_lossy().into_owned()); + app.save_override = Some(dir.join("contract_signed.pdf").to_string_lossy().into_owned()); + app + }); + h.run_steps(6); + h +} + +#[test] +fn one_click_trust_stores_the_displayed_signer_not_the_embedded_root() { + let dir = dir(); + let mut h = harness(dir); + // Sign with chain.p12, whose CMS embeds the signer's issuer chain (signer + self-signed root). + let p12 = concat!(env!("CARGO_MANIFEST_DIR"), "/../sign/tests/data/chain.p12"); + let id = pdfcraft_engine::sign::pkcs12::open(&std::fs::read(p12).unwrap(), "test").unwrap(); + let entry = h.state_mut().add_digital_id(p12, &id.certificate); + h.state_mut().start_signing(0, None, None, None); + { + let d = h.state_mut().sign_draft.as_mut().unwrap(); + d.selected = Some(entry); + d.step = SignStep::SignAs; + d.password = "test".into(); + } + h.run_steps(2); + h.get_by_label("Sign").click(); + h.run_steps(6); + + // The signature validates as Unknown and carries a two-certificate chain whose top (the root) + // is a different certificate from the displayed signer. + let (signer, root) = { + let s = h.state(); + let sig = s.session.get(s.views[0].id).unwrap().signatures.iter().find(|x| x.signed).cloned().expect("a signed field"); + assert_eq!(sig.status, SignatureStatus::Unknown, "untrusted before the click"); + assert_eq!(sig.chain.len(), 2, "the CMS embeds the signer and its root: {:?}", sig.chain.len()); + let signer = sig.certificate.clone().expect("a signer certificate"); + let root = sig.chain.last().cloned().expect("a chain top"); + assert_ne!(signer.raw, root.raw, "the embedded chain top must differ from the signer for this test to be meaningful"); + (signer, root) + }; + + // Expand the signature row and use the one-click trust button. + h.get_by_label_contains("Signed by").click(); + h.run_steps(2); + h.get_by_label("Add to trusted certificates").click(); + h.run_steps(3); + + // The trust store holds exactly the displayed signer, never the document-supplied root. + let trusted = h.state().session.trusted_certificates().to_vec(); + assert!(trusted.iter().any(|c| c.raw == signer.raw), "the displayed signer should be trusted"); + assert!(!trusted.iter().any(|c| c.raw == root.raw), "the embedded root must not be trusted by the one-click action"); +} diff --git a/crates/xfa/src/layout.rs b/crates/xfa/src/layout.rs index 13712e910..ee8af4cf1 100644 --- a/crates/xfa/src/layout.rs +++ b/crates/xfa/src/layout.rs @@ -30,6 +30,10 @@ const MAX_MEASURES: usize = 2_000_000; const MAX_INSTANCES: usize = 50; /// Largest picture taken from the data (decoded bytes), as for template pictures. const MAX_DATA_IMAGE: usize = 32 << 20; +/// Most picture bytes one layout keeps, all copies together. Each laid-out picture holds its +/// own copy, so a large picture in repeating subforms would otherwise multiply into gigabytes +/// (a failed allocation aborts the app); a real form's logos and photos stay far below. +const MAX_IMAGE_BYTES: usize = 256 << 20; /// Placeholder for the page count in text laid out before the count is known. const PAGE_COUNT_MARK: char = '\u{E000}'; @@ -195,6 +199,8 @@ struct Layouter<'t> { warned: HashSet, fields: usize, items: usize, + /// Picture bytes laid out so far (see [`MAX_IMAGE_BYTES`]). + image_bytes: usize, /// Container sizes by (address of the subform or area in the template, available width, /// depth), with the inherited font and paragraph they were measured with. measured: RefCell>, @@ -249,6 +255,7 @@ pub fn layout(tpl: &Template, data: Option<&DataNode>) -> Result warned: HashSet::new(), fields: 0, items: 0, + image_bytes: 0, measured: RefCell::new(HashMap::new()), measures: Cell::new(0), leaves: Vec::new(), @@ -1383,6 +1390,10 @@ impl Layouter<'_> { /// A picture scaled to fit `inner`, keeping its shape, from its top left. fn draw_image(&mut self, data: &[u8], content_type: &str, inner: Rect) -> Result<(), XfaError> { + self.image_bytes = self.image_bytes.saturating_add(data.len()); + if self.image_bytes > MAX_IMAGE_BYTES { + return Err(XfaError::TooLarge("the form's pictures are too large together to lay out".into())); + } let fitted = match crate::image::size(data) { Some((iw, ih)) if iw > 0 && ih > 0 && inner.w > 0.0 && inner.h > 0.0 => { let k = (inner.w / iw as f64).min(inner.h / ih as f64); diff --git a/crates/xfa/src/tests.rs b/crates/xfa/src/tests.rs index 91fcf7789..24260ec03 100644 --- a/crates/xfa/src/tests.rs +++ b/crates/xfa/src/tests.rs @@ -222,6 +222,29 @@ fn hostile_templates_fail_without_panicking() { assert!(!f.warnings.is_empty()); } +#[test] +fn repeated_pictures_are_bounded_in_total_bytes() { + // Each laid-out picture keeps its own copy of the bytes, so one picture in a repeating + // subform used to multiply without bound and exhaust memory (a failed allocation aborts the + // app). "AAAA" base64 decodes to zero bytes; 2_097_152 groups is 6 MiB, under the 32 MiB + // per-picture cap. 50 instances (occur initial, clamped to MAX_INSTANCES) would be ~300 MiB, + // past the aggregate cap, so layout must stop with TooLarge rather than retain it all. + let big = "AAAA".repeat(2_097_152); + let tpl = format!( + "" + ); + assert!(matches!(layout_xml(&tpl), Err(XfaError::TooLarge(m)) if m.contains("too large")), "the repeated picture must be refused"); + + // A single small picture still lays out: the cap does not reject an ordinary form. + let small = "AAAA".repeat(8); + let ok = format!( + "" + ); + assert!(layout_xml(&ok).is_ok(), "one small picture must lay out"); +} + #[test] fn the_packets_can_be_one_stream_or_named_parts() { // Named parts, with the template split from the rest. diff --git a/crates/xfdf/src/lib.rs b/crates/xfdf/src/lib.rs index 7bec9c5f4..4998e7543 100644 --- a/crates/xfdf/src/lib.rs +++ b/crates/xfdf/src/lib.rs @@ -754,7 +754,27 @@ fn annot_from_xml(node: roxmltree::Node, subtype: &str, page_ref: ObjRef) -> Opt /// Put an annotation dictionary on a page, replacing one with the same `/NM`. Returns its /// reference. -fn place(doc: &mut Document, page_ref: ObjRef, d: Dict) -> Result { +/// `/F` bits an imported file may not set on a redaction mark: Invisible, Hidden, NoView — so a +/// mark it brings in is always drawn for the user to review before content is removed. +const REDACT_UNSEEN: i64 = 1 | 2 | 32; +/// `/F` Locked (ISO 32000-2 Table 167): the user protected this annotation from change. +const LOCKED: i64 = 128; + +/// An existing annotation an import must never replace, move or unlock: one the user locked, or a +/// redaction mark (replacing it would drop the region the user marked to remove). +fn import_protected(x: &Dict) -> bool { + x.int(b"F").unwrap_or(0) & LOCKED != 0 || x.name(b"Subtype") == Some(b"Redact".as_slice()) +} + +fn place(doc: &mut Document, page_ref: ObjRef, mut d: Dict) -> Result { + // A redaction mark the import carries is always made visible: clear any Invisible/Hidden/NoView + // flag so it cannot remove content the user never sees. + if d.name(b"Subtype") == Some(b"Redact".as_slice()) { + let f = d.int(b"F").unwrap_or(0); + if f & REDACT_UNSEEN != 0 { + d.set(b"F".to_vec(), Object::Int(f & !REDACT_UNSEEN)); + } + } let nm = d.get(b"NM").and_then(|n| n.as_string().map(|s| s.to_text())); let annots: Vec = doc .get(page_ref) @@ -765,15 +785,24 @@ fn place(doc: &mut Document, page_ref: ObjRef, d: Dict) -> Result = Vec::new(); + // Never let a same-name import replace a locked comment or a redaction mark: keep it, and add + // the imported dict beside it under no name of its own. + let mut clash = false; for a in annots { - let same = nm.is_some() - && a.as_ref().and_then(|r| doc.get(r).as_dict().and_then(|x| x.get(b"NM").and_then(|n| n.as_string().map(|s| s.to_text())))) == nm; - if same { + let existing = a.as_ref().and_then(|r| doc.get(r).as_dict().cloned()); + let same = nm.is_some() && existing.as_ref().and_then(|x| x.get(b"NM").and_then(|n| n.as_string().map(|s| s.to_text()))) == nm; + if same && existing.as_ref().is_some_and(import_protected) { + clash = true; + kept.push(a); + } else if same { removed.extend(a.as_ref()); } else { kept.push(a); } } + if clash { + d.remove(b"NM"); + } // The replaced note's pop-up goes with it; otherwise it stays on the page pointing at a note that is no longer there. kept.retain(|a| { let Some(x) = a.as_ref().map(|r| doc.get(r)).and_then(|o| o.as_dict().cloned()) else { return true }; diff --git a/crates/xfdf/src/tests.rs b/crates/xfdf/src/tests.rs index 0f32cced8..31e7f5c3f 100644 --- a/crates/xfdf/src/tests.rs +++ b/crates/xfdf/src/tests.rs @@ -399,6 +399,58 @@ fn reimporting_a_note_replaces_its_popup() { assert!(refs.contains(&parent), "the pop-up belongs to a note that is still on the page"); } +#[test] +fn imported_redact_marks_cannot_replace_a_locked_mark_or_hide_themselves() { + let mut doc = blank(); + let page = pdfcraft_model::pages(&doc)[0].obj; + // A locked redaction mark the user placed, with a known /NM. + let mut mark = Dict::new(); + mark.set(b"Type".to_vec(), Object::name("Annot")); + mark.set(b"Subtype".to_vec(), Object::name("Redact")); + mark.set(b"NM".to_vec(), PdfString::text("mark1")); + mark.set(b"Rect".to_vec(), Object::Array(vec![60.into(), 660.into(), 560.into(), 678.into()])); + mark.set(b"F".to_vec(), Object::Int(128)); // Locked + let mark_ref = doc.add(Object::Dict(mark)); + let mut annots = + doc.get(page).as_dict().and_then(|p| p.get(b"Annots").cloned()).and_then(|a| doc.resolve(&a).as_array().cloned()).unwrap_or_default(); + annots.push(Object::Ref(mark_ref)); + doc.update_dict(page, |p| p.set(b"Annots".to_vec(), Object::Array(annots))).unwrap(); + + // A reviewer's XFDF: shrink the locked mark to a 1×1 corner by reusing its /NM, and add a + // redaction mark imported as Hidden that would remove other content unseen. + let xfdf = concat!( + r#""#, + r#""#, + r#""#, + r#""#, + ); + import(&mut doc, xfdf.as_bytes()).unwrap(); + + let refs: Vec = doc + .get(page) + .as_dict() + .and_then(|p| p.get(b"Annots").cloned()) + .and_then(|a| doc.resolve(&a).as_array().cloned()) + .unwrap_or_default() + .iter() + .filter_map(|a| a.as_ref()) + .collect(); + // The locked mark is still there, unchanged (same object, same geometry, still its /NM). + assert!(refs.contains(&mark_ref), "the locked redaction mark was replaced"); + let locked = doc.get(mark_ref).as_dict().cloned().unwrap(); + assert_eq!(nums_of(&doc, locked.get(b"Rect")), vec![60.0, 660.0, 560.0, 678.0], "the locked mark's geometry was changed"); + assert_eq!(locked.get(b"NM").and_then(|n| n.as_string().map(|s| s.to_text())).as_deref(), Some("mark1")); + assert_eq!(locked.int(b"F"), Some(128), "the locked mark lost its Locked flag"); + // Every redaction mark on the page is visible (none Invisible/Hidden/NoView): the import's two + // marks were added beside the protected one, both drawable for review. + let redacts: Vec = + refs.iter().map(|r| doc.get(*r).as_dict().cloned().unwrap()).filter(|d| d.name(b"Subtype") == Some(b"Redact".as_slice())).collect(); + assert_eq!(redacts.len(), 3, "the imported marks should be added, not merged away"); + for d in &redacts { + assert_eq!(d.int(b"F").unwrap_or(0) & (1 | 2 | 32), 0, "a redaction mark is hidden from review: {d:?}"); + } +} + /// The `/Vertices` of every annotation on the document's pages, by subtype. fn vertices(doc: &Document) -> Vec<(String, Vec)> { let mut out = Vec::new(); @@ -492,6 +544,42 @@ fn xfdf_keeps_callout_leader_inset_and_ending() { assert_eq!(callout_keys(&dst), want, "{xfdf}"); } +/// Run `f` on a thread with a small stack, as a deep recursion would overflow it. +fn on_small_stack(f: impl FnOnce() -> T + Send + 'static) -> T { + std::thread::Builder::new().stack_size(256 << 10).spawn(f).unwrap().join().unwrap() +} + +#[test] +fn deeply_nested_xml_is_refused_before_parsing() { + // A file nested far deeper than any stack holds used to overflow roxmltree's recursive + // parser, which aborts the process. It must be refused without recursing. + let deep = |root: &str| format!("<{root}>{}{}", "".repeat(100_000), "".repeat(100_000)); + let xfdf = deep("xfdf"); + let xml = deep("fields"); + let (imported, table, values) = on_small_stack(move || { + let mut doc = blank(); + (import(&mut doc, xfdf.as_bytes()), import(&mut doc, xml.as_bytes()), data_values(xfdf.as_bytes())) + }); + for r in [imported.map(|_| ()), table.map(|_| ()), values.map(|_| ())] { + assert!(matches!(&r, Err(DataError::Malformed(m)) if m.contains("deep")), "{r:?}"); + } +} + +#[test] +fn wide_or_commented_xml_is_not_mistaken_for_deep() { + // Thousands of siblings, self-closing tags, `>` in attribute values, and tags inside + // comments, CDATA and processing instructions add no depth. + let mut x = String::from(""); + for i in 0..5_000 { + x.push_str(&format!(" g\">v]]> ?>")); + } + x.push_str(""); + assert_eq!(check_form_xml_depth(&x), Ok(())); + let nested = |n: usize| format!("{}{}", "".repeat(n), "".repeat(n)); + assert_eq!(check_form_xml_depth(&nested(MAX_XML_DEPTH)), Ok(())); + assert!(check_form_xml_depth(&nested(MAX_XML_DEPTH + 1)).is_err()); +} + #[test] fn duplicate_xfdf_annotation_flags_do_not_turn_on_unrelated_bits() { // Flags are a set of bit masks, not additive integers. A repeated "print" flag