fix(preflight): check the actions an allowed action chains to

Conversion and verification looked only at an action's own /S. An allowed
action (a GoTo, say) whose /Next held a JavaScript or other forbidden
action written directly inside it was kept, and verification never looked
at actions written directly in a catalog /OpenAction or an annotation /A.
Both now follow /Next through the actions written inside an action (an
indirect /Next is an object of its own, checked as one), within a bound,
and conversion removes the whole chain.
This commit is contained in:
aiongg committed 2026-10-22 17:57:53 +00:00
1 parent adab150a45
commit 90982ba9a8
2 files changed
+76 -13

No files matched your search

+50 -13
View File
@@ -515,12 +515,14 @@ pub fn verify(doc: &Document, level: Level) -> Vec<Issue> {
let Some(d) = dict_of(o) else { continue };
let ty = d.name(b"Type");
let sub = d.name(b"Subtype");
// Actions anywhere (structure elements also have /S: their role).
if let Some(s) = d.name(b"S")
&& FORBIDDEN_ACTIONS.contains(&s)
&& is_action(d)
{
issue("6.5.1", "A {} action isn't allowed", Some(String::from_utf8_lossy(s).into_owned()), None, true);
// Actions anywhere (structure elements also have /S: their role), with the actions they
// chain to and those written directly in an /A or /OpenAction.
let own = (d.get(b"S").is_some() && is_action(d)).then_some(d);
let direct = [b"A".as_slice(), b"OpenAction"].map(|k| d.get(k).and_then(|a| a.as_dict()));
for a in [own].into_iter().chain(direct).flatten() {
if let Some(s) = chain_forbidden(a) {
issue("6.5.1", "A {} action isn't allowed", Some(s), None, true);
}
}
// Annotations.
if ty == Some(b"Annot") || (sub.is_some() && d.get(b"Rect").is_some() && d.get(b"P").is_some()) {
@@ -652,10 +654,39 @@ fn is_action(d: &Dict) -> bool {
}
}
/// Whether `o` is (or refers to) a forbidden action, or to nothing.
/// How many actions one action and the `/Next` actions written inside it may hold before the
/// whole chain counts as forbidden, since it can't be checked.
const MAX_ACTION_CHAIN: usize = 1024;
/// The first forbidden action type in the action `a` or in the actions written directly in its
/// `/Next` (ISO 32000-2 12.6.2: a dictionary or an array of them). An action `/Next` refers to
/// is an object of its own, checked and removed as one.
fn chain_forbidden(a: &Dict) -> Option<String> {
let mut stack = vec![a];
let mut read = 0;
while let Some(d) = stack.pop() {
read += 1;
if read > MAX_ACTION_CHAIN {
return Some("too deeply chained".into());
}
if let Some(s) = d.name(b"S")
&& FORBIDDEN_ACTIONS.contains(&s)
{
return Some(String::from_utf8_lossy(s).into_owned());
}
match d.get(b"Next") {
Some(Object::Dict(next)) => stack.push(next),
Some(Object::Array(list)) => stack.extend(list.iter().filter_map(Object::as_dict)),
_ => {}
}
}
None
}
/// Whether `o` is (or refers to) a forbidden action or one chaining to it, or to nothing.
fn forbidden(doc: &Document, o: &Object) -> bool {
match &*doc.resolve(o) {
Object::Dict(a) => a.name(b"S").is_some_and(|s| FORBIDDEN_ACTIONS.contains(&s)),
Object::Dict(a) => chain_forbidden(a).is_some(),
Object::Null => true,
_ => false,
}
@@ -801,14 +832,20 @@ pub fn convert(doc: &mut Document, level: Level) -> Result<Report, pdfcraft_cos:
files += 1;
}
}
// Forbidden actions left as objects of their own (nothing refers to them any more).
// Forbidden actions left as objects of their own, or chained to by one, and those written
// directly in an /A elsewhere (an outline item's, say).
for (r, o) in objects(doc) {
if let Object::Dict(d) = &*o
&& d.name(b"S").is_some_and(|s| FORBIDDEN_ACTIONS.contains(&s))
&& is_action(d)
{
let Object::Dict(d) = &*o else { continue };
if d.get(b"S").is_some() && is_action(d) && chain_forbidden(d).is_some() {
doc.set(r, Object::Null);
actions += 1;
} else if let Some(Object::Dict(a)) = d.get(b"A")
&& chain_forbidden(a).is_some()
{
let mut nd = d.clone();
nd.remove(b"A");
doc.set(r, Object::Dict(nd));
actions += 1;
}
}
for (n, what) in [
+26
View File
@@ -430,6 +430,32 @@ fn content_colour_reads_operators_one_at_a_time_and_stops_when_all_are_found() {
assert_eq!(read, 3);
}
/// A forbidden action chained by `/Next` behind an allowed one, written directly inside its
/// parent, is reported and removed like any other.
#[test]
fn forbidden_actions_chained_behind_allowed_ones_are_found_and_removed() {
let js = "<< /S /JavaScript /JS (app.alert(1)) >>";
let page = "<< /Type /Page /Parent 2 0 R /Annots [4 0 R] >>";
let pages = "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 200 200] >>";
let link = |a: &str| format!("<< /Type /Annot /Subtype /Link /Rect [0 0 0 0] /P 3 0 R /F 4 /A {a} >>");
let open_action = format!("<< /Type /Catalog /Pages 2 0 R /OpenAction << /S /GoTo /D [3 0 R /Fit] /Next {js} >> >>");
let goto = format!("<< /Type /Action /S /GoTo /D [3 0 R /Fit] /Next [{js}] >>");
let cases = [
build(&[&open_action, pages, page, &link("<< /S /GoTo /D [3 0 R /Fit] >>")], ""),
build(&["<< /Type /Catalog /Pages 2 0 R >>", pages, page, &link("5 0 R"), &goto], ""),
build(&["<< /Type /Catalog /Pages 2 0 R >>", pages, page, &link(&format!("<< /S /GoTo /D [3 0 R /Fit] /Next {js} >>"))], ""),
];
for (i, mut doc) in cases.into_iter().enumerate() {
let found = messages(&verify(&doc, Level::A2b)).contains(&"A JavaScript action isn't allowed");
assert!(found, "case {i}: {:#?}", verify(&doc, Level::A2b));
let r = convert(&mut doc, Level::A2b).unwrap();
assert!(!r.remaining.iter().any(|x| x.clause == "6.5.1"), "case {i}: {:#?}", r.remaining);
let plain = pdfcraft_cos::SaveOptions { object_streams: false, ..Default::default() };
let bytes = pdfcraft_cos::write_full(&doc, &plain).unwrap();
assert!(!bytes.windows(11).any(|w| w == b"/JavaScript"), "case {i}: {}", String::from_utf8_lossy(&bytes));
}
}
/// Each annotation's page is found from one pass over the pages' `/Annots`, not by searching
/// every page's list again for each annotation.
#[test]