fix(cos): encrypt the /Contents of an annotation that carries a /ByteRange

Saving with encryption left a dictionary's /Contents in clear whenever it
had a string /Contents and an array /ByteRange, whatever else it was. A
source file could add a /ByteRange to an ordinary annotation and its
comment text was then written unencrypted. A dictionary with a /Subtype or
a /Type other than Sig or DocTimeStamp is never a signature dictionary, so
its /Contents is encrypted like any other string.

crates/cos/src/security.rs

@@ -88,10 +88,17 @@ fn named_crypt_filter(dict: &Dict) -> Option<Vec<u8>> {
 /// whose `/Contents` is never encrypted (ISO 32000-2 §7.6.2, §12.8.1): the signer writes a
 /// placeholder, hashes the file around it and patches the value in place, so encrypting it
 /// would make signing impossible. Identified as other readers do: by `/Type`, or by a string
-/// `/Contents` alongside an array `/ByteRange` (`/Type` is optional).
+/// `/Contents` alongside an array `/ByteRange` (`/Type` is optional). A signature dictionary
+/// has no `/Subtype`; a dictionary with one (an annotation, whose `/Contents` is the comment
+/// text) or with another `/Type` is never exempt, whatever else it carries.
 fn is_signature_dict(d: &Dict) -> bool {
-    matches!(d.name(b"Type"), Some(b"Sig" | b"DocTimeStamp"))
-        || (matches!(d.get(b"Contents"), Some(Object::String(_))) && matches!(d.get(b"ByteRange"), Some(Object::Array(_))))
+    if d.get(b"Subtype").is_some() {
+        return false;
+    }
+    match d.get(b"Type") {
+        Some(_) => matches!(d.name(b"Type"), Some(b"Sig" | b"DocTimeStamp")),
+        None => matches!(d.get(b"Contents"), Some(Object::String(_))) && matches!(d.get(b"ByteRange"), Some(Object::Array(_))),
+    }
 }
 
 /// Decrypt (`decrypt = true`) or encrypt every string and stream in an indirect object,

crates/cos/tests/it/encryption.rs

@@ -279,6 +279,38 @@ fn signature_contents_stay_unencrypted() {
     }
 }
 
+/// Only a signature dictionary keeps its `/Contents` in clear. An annotation that carries a
+/// `/ByteRange` (or even `/Type /Sig`) is still an annotation, and its comment is encrypted.
+#[test]
+fn a_byte_range_does_not_exempt_an_annotation() {
+    for (ty, subtype) in [(Some("Annot"), "Text"), (None, "FreeText"), (Some("Sig"), "Text")] {
+        let mut doc = Document::open(Arc::new(fixture())).unwrap();
+        let mut note = pdfcraft_cos::Dict::new();
+        if let Some(ty) = ty {
+            note.set(b"Type".to_vec(), Object::name(ty));
+        }
+        note.set(b"Subtype".to_vec(), Object::name(subtype));
+        note.set(b"Contents".to_vec(), PdfString::literal(b"Private note".to_vec()));
+        note.set(b"ByteRange".to_vec(), Object::Array(Vec::new()));
+        let note = doc.add(Object::Dict(note));
+        doc.update_dict(doc.root().unwrap(), |c| c.set(b"Test".to_vec(), Object::Ref(note))).unwrap();
+        doc.set_encryption(&NewEncryption {
+            algorithm: Algorithm::Aes256,
+            user_password: "",
+            owner_password: "owner",
+            permissions: -1,
+            encrypt_metadata: true,
+            seed: [3; 32],
+        })
+        .unwrap();
+        let bytes = write_full(&doc, &SaveOptions { object_streams: false, ..SaveOptions::default() }).unwrap();
+        assert!(!String::from_utf8_lossy(&bytes).contains("Private note"), "{ty:?} {subtype}: comment encrypted");
+        let back = Document::open(Arc::new(bytes)).unwrap();
+        let note = back.get(back.root().unwrap()).as_dict().unwrap().reference(b"Test").unwrap();
+        assert_eq!(back.get(note).as_dict().unwrap().get(b"Contents").unwrap().as_string().unwrap().bytes, b"Private note");
+    }
+}
+
 #[test]
 fn a_nonpositive_crypt_filter_length_reads_as_the_default_key_length() {
     // A V4 crypt filter's /Length counts bytes (≤ 32) or bits. A zero or negative value is