Security: fix 10 verified audit findings in redaction, signature validation, and untrusted-input handling (#1099)

* fix(create): cap decoded size of imported PNGs

A PNG's declared dimensions sized an infallible allocation, so a few-byte file claiming huge dimensions aborted the process when the allocation failed. Cap the decoded size at the TIFF ceiling and reserve every input-sized buffer fallibly, returning CreateError::Image instead. (Security audit run-2: v3-024.)

Co-Authored-By: Claude Opus 4.8 <[email protected]>

* fix(xfa): bound total picture bytes kept during layout

Each laid-out picture kept its own copy, so one picture in a repeating subform multiplied without bound and could exhaust memory on open. Add a per-layout aggregate cap (256 MiB) that stops with XfaError::TooLarge. (Security audit run-2: v3-103.)

Co-Authored-By: Claude Opus 4.8 <[email protected]>

* fix(xfdf): bound XML nesting and protect redaction marks on import

Deeply nested XFDF/XML overflowed roxmltree's recursive parser and aborted the process; a non-recursive depth scan now rejects it before parsing. Imported comments could also replace a locked or redaction mark by /NM or add a hidden one; place() no longer replaces locked/Redact annotations and clears the hidden/invisible flags on imported redaction marks. (Security audit run-2: v3-105, v3-111.)

Co-Authored-By: Claude Opus 4.8 <[email protected]>

* fix(sign): classify post-signing changes by the signed revision's roles

A signed object's modification was classified from labels in the attacker-written new version, so relabelling a signed page /Type /Sig (or giving it /ModDate), or relabelling a drawn stream /ObjStm or /XRef, let a forged page validate as Valid even under DocMDP P=1. Decide an object's role (page content, signature, metadata, file plumbing) from what it was in the signed revision, not its new dictionary. (Security audit run-2: v3-082, v3-083.)

Co-Authored-By: Claude Opus 4.8 <[email protected]>

* fix(redact): remove redacted content from the saved file, fail closed otherwise

Apply Redactions and Remove Hidden Information left the originals behind: dropped or cleaned image/form XObjects kept their resource names (so the full rewrite saved the originals), an XFA form kept the redacted value in its datasets packet, and vertical-writing text under a mark was never hit by the horizontal removal geometry yet reported success. Prune replaced XObject names the page no longer draws, drop /XFA and /NeedsRendering when redacting an XFA form, and fail closed (RedactError::VerticalText) rather than silently leave vertical text. (Security audit run-2: v3-064, v3-067, v3-062.)

Co-Authored-By: Claude Opus 4.8 <[email protected]>

* fix(ui-egui): trust the displayed signer, not the document's embedded root

The Signatures panel's one-click 'Add to trusted certificates' trusted the top of the chain the document supplied, installing the author's self-signed root as an anchor instead of the signer shown. Trust the displayed signer certificate. (Security audit run-2: v3-098.)

Co-Authored-By: Claude Opus 4.8 <[email protected]>

* Use main's decoded-size ceiling for PNGs too; keep the depth tests on main's checker

---------

Co-authored-by: Claude Opus 4.8 <[email protected]>
Co-authored-by: Brandon Thomas <[email protected]>
This commit is contained in:
authored and GitHub committed 2026-10-11 15:09:39 -04:00
1 parent 8f8e0236aa
commit b642e327e7
14 files changed
+736 -35

No files matched your search

+13 -4
View File
@@ -314,10 +314,13 @@ fn png_image(name: &str, bytes: &[u8]) -> Result<Embedded, CreateError> {
_ => (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<Embedded, CreateError> {
};
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 {
+39
View File
@@ -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<u8> {
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]
+84 -9
View File
@@ -38,6 +38,10 @@ pub(crate) enum Mode {
struct Gs {
ctm: Matrix,
font: Rc<Metrics>,
/// 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<Option<Vec<u8>>>,
/// New XObjects the rewritten content refers to: (resource name, object).
pub xobjects: Vec<(Vec<u8>, 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<Vec<u8>>,
/// 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<u8> {
let mut i = scope.used_names.len() + 1;
loop {
@@ -160,6 +185,23 @@ fn fresh_name(scope: &mut Scope<'_>, xobjects: &Dict) -> Vec<u8> {
}
}
/// 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<u8>], content: &[Vec<u8>]) {
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<u8>], 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<Gs> = 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)])))
}
+55 -2
View File
@@ -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<Report, Reda
let mut scope = Scope::new(&rects, Mode::Apply, &mut report);
process(doc, &mut scope, &data, &resources, pdfcraft_content::Matrix::IDENTITY)
};
// Fail closed: the interpreter lays vertical-writing text out horizontally, so it cannot
// place such glyphs under a mark or confirm they are gone. Rather than report success and
// ship the text under the box, refuse the whole operation (the caller keeps the previous
// document). Full vertical-text removal is a future enhancement.
if out.vertical {
return Err(RedactError::VerticalText(pi + 1));
}
let mut new_list = list.clone();
let mut changed = false;
for (i, new) in out.streams.into_iter().enumerate() {
@@ -309,10 +321,20 @@ pub fn apply(doc: &mut Document, pages: Option<&[usize]>) -> Result<Report, Reda
new_list[i] = Object::Ref(doc.add(Object::Stream(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 image or form bytes cannot survive in the saved file.
let page_content: Vec<Vec<u8>> = 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<Report, Reda
}
}
// 3b. An XFA form mirrors its field values in the /XFA datasets packet, and a relayout
// rebuilds fields and pages from that packet, so deleting the AcroForm field alone leaves the
// redacted value in the file and lets a later relayout put it back. Once redactions are
// applied the form stops being XFA: drop /AcroForm /XFA (template and datasets), the catalog's
// /NeedsRendering, and PdfCraft's own layout mark (pdfcraft_xfa::LAYOUT_KEY), leaving a plain
// already-laid-out AcroForm that neither the saved file nor a relayout can restore from.
if let Some(root) = doc.root() {
let strip_xfa = |d: &mut Dict| {
d.remove(b"XFA");
d.remove(b"PCXfaLayout");
};
match doc.get(root).as_dict().and_then(|d| d.get(b"AcroForm").cloned()) {
Some(Object::Ref(af)) if doc.get(af).as_dict().is_some_and(|d| d.contains(b"XFA")) => {
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();
+12 -2
View File
@@ -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<Vec<u8>> = 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));
}
+130
View File
@@ -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<u8>> = 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<u8>> = 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"<xfa:datasets xmlns:xfa=\"http://www.xfa.org/schema/xfa-data/1.0/\"><xfa:data><ssn>SSN-987-65-4321</ssn></xfa:data></xfa:datasets>",
),
stream(
"",
b"<template xmlns=\"http://www.xfa.org/schema/xfa-template/3.3/\"><subform name=\"form1\"><field name=\"ssn\"/></subform></template>",
),
];
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![]);
+59 -13
View File
@@ -976,6 +976,12 @@ fn classify_changes(doc: &Document, old: Option<Document>, p: Option<u8>) -> 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<ObjRef> = 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<Document>, p: Option<u8>) -> 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<u8>, Object)> {
let mut v: Vec<(Vec<u8>, 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<ObjRef> = 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<u8>, Object)> {
let mut v: Vec<(Vec<u8>, 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";
+98
View File
@@ -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<u8> {
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<u8> {
let alg = id.key.signature_algorithm(pdfcraft_sign::DigestAlg::Sha256);
+4 -1
View File
@@ -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()
{
+87
View File
@@ -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");
}
+11
View File
@@ -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<String>,
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<HashMap<MeasureKey, Measured>>,
@@ -249,6 +255,7 @@ pub fn layout(tpl: &Template, data: Option<&DataNode>) -> Result<Form, XfaError>
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);
+23
View File
@@ -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!(
"<template><subform layout=\"tb\"><subform layout=\"tb\"><occur initial=\"50\" max=\"-1\"/>\
<draw w=\"0.2in\" h=\"0.2in\"><value><image contentType=\"image/png\">{big}</image></value></draw>\
</subform></subform></template>"
);
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!(
"<template><subform layout=\"tb\"><draw w=\"1in\" h=\"1in\"><value><image contentType=\"image/png\">{small}</image></value></draw></subform></template>"
);
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.
+33 -4
View File
@@ -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<ObjRef, DataError> {
/// `/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<ObjRef, DataError> {
// 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<Object> = doc
.get(page_ref)
@@ -765,15 +785,24 @@ fn place(doc: &mut Document, page_ref: ObjRef, d: Dict) -> Result<ObjRef, DataEr
.unwrap_or_default();
let mut kept = Vec::new();
let mut removed: Vec<ObjRef> = 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 };
+88
View File
@@ -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#"<xfdf xmlns="http://ns.adobe.com/xfdf/"><annots>"#,
r#"<redact page="0" name="mark1" rect="0,0,1,1" title="Reviewer"/>"#,
r#"<redact page="0" name="ghost" rect="60,600,560,620" flags="hidden" title="Reviewer"/>"#,
r#"</annots></xfdf>"#,
);
import(&mut doc, xfdf.as_bytes()).unwrap();
let refs: Vec<ObjRef> = 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<Dict> =
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<f64>)> {
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<T: Send + 'static>(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}>{}{}</{root}>", "<a>".repeat(100_000), "</a>".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("<?xml version=\"1.0\"?><!-- <a><a><a> --><xfdf><fields>");
for i in 0..5_000 {
x.push_str(&format!("<field name=\"f{i} > g\"><value>v</value></field><empty/><![CDATA[<b><b>]]><?pi <c> ?>"));
}
x.push_str("</fields></xfdf>");
assert_eq!(check_form_xml_depth(&x), Ok(()));
let nested = |n: usize| format!("{}{}", "<a>".repeat(n), "</a>".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