M6: misc hardening — unbounded picture cache, stats walk, signing tmpdir, web clipboard, dash budget, fs cycle, rezip loop

- floating_pictures: draw without retaining when eviction can't free
  enough room, so a large workbook can't pin the cache past its budget
- xtask stats: skip unreadable subtrees instead of aborting the walk
- import-cert.sh: per-run private temp dir instead of predictable
  paths in a shared root
- gridcraft-web: gate clipboard ops behind user activation
- chart raster: cap dash rendering to the stroked budget
- cli check: mark directories visited by identity on unix so hard-link
  loops can't recurse; reuse one decompression of a reused part

apps/gridcraft-cli/src/check.rs

@@ -248,10 +248,11 @@ impl Discovery {
 /// (`~$…`) are skipped.
 fn collect_inputs(inputs: &[String]) -> Discovery {
     let mut out = Discovery::default();
+    let mut seen = std::collections::HashSet::new();
     for i in inputs {
         let p = PathBuf::from(i);
         if p.is_dir() {
-            if !walk(&p, 0, &mut out) {
+            if !walk(&p, 0, &mut out, &mut seen) {
                 break;
             }
         } else if !out.add_file(p) {
@@ -268,7 +269,20 @@ fn collect_inputs(inputs: &[String]) -> Discovery {
 }
 
 /// Returns false only when the global file budget stops traversal.
-fn walk(dir: &Path, depth: usize, out: &mut Discovery) -> bool {
+fn walk(dir: &Path, depth: usize, out: &mut Discovery, seen: &mut std::collections::HashSet<u64>) -> bool {
+    // A directory is visited once by identity (dev+inode on unix): several links to the same
+    // directory are aliases, not exponential branches of the scan. Other platforms keep the
+    // depth cap only.
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::MetadataExt as _;
+        if let Ok(m) = std::fs::metadata(dir)
+            && let Some(id) = m.dev().checked_mul(1 << 60).and_then(|d| d.checked_add(m.ino()))
+            && !seen.insert(id)
+        {
+            return true;
+        }
+    }
     if depth > MAX_DEPTH {
         out.fail(dir, format!("directory depth limit of {MAX_DEPTH} exceeded; this subtree was not checked"));
         return true;
@@ -301,7 +315,7 @@ fn walk(dir: &Path, depth: usize, out: &mut Discovery) -> bool {
             }
         };
         if metadata.is_dir() {
-            if !walk(&p, depth + 1, out) {
+            if !walk(&p, depth + 1, out, seen) {
                 return false;
             }
         } else if p.extension().and_then(|e| e.to_str()).is_some_and(|e| e.eq_ignore_ascii_case("xlsx") || e.eq_ignore_ascii_case("xlsm"))
@@ -981,6 +995,12 @@ fn relationship_kinds(bytes: &[u8]) -> Result<BTreeMap<String, String>, String>
     let mut zip = zip::ZipArchive::new(Cursor::new(bytes)).map_err(|e| format!("not a zip package: {e}"))?;
     let names = zip_names(&mut zip);
     let by_key: HashMap<String, &String> = names.iter().map(|n| (part_key(n), n)).collect();
+    // Several relationships name the same part; each fresh check decompressed it again, so
+    // repeated references to one highly compressible part multiplied the work. The verdict is
+    // remembered per (part, named content) and the decompression the package can cause is
+    // budgeted as a whole (finding: repeated decompression).
+    let mut verdicts: HashMap<(String, &'static str), bool> = HashMap::new();
+    let mut budget: u64 = 512 * 1024 * 1024;
     let mut kinds = BTreeMap::new();
     for n in names.iter().filter(|n| part_key(n).ends_with(".rels")) {
         let Some(src_dir) = rels_source_dir(&part_key(n)) else { continue };
@@ -994,7 +1014,7 @@ fn relationship_kinds(bytes: &[u8]) -> Result<BTreeMap<String, String>, String>
             let external = attr(attrs, "TargetMode").is_some_and(|m| m.eq_ignore_ascii_case("External"));
             if !external
                 && let Some(part) = by_key.get(&resolve_target(&src_dir, target).to_ascii_lowercase())
-                && !holds_something(&mut zip, part, kind)
+                && !holds_something(&mut zip, part, kind, &mut verdicts, &mut budget)
             {
                 continue;
             }
@@ -1013,15 +1033,41 @@ fn rels_source_dir(key: &str) -> Option<String> {
 
 /// Whether a part has content: an element under its root, or for comments (whose authors list
 /// comes first) a comment, for VML a shape. Read only as far as needed; an unreadable part counts.
-fn holds_something(zip: &mut Zip<'_>, name: &str, kind: &str) -> bool {
-    use quick_xml::events::Event;
-    let Ok(f) = zip.by_name(name) else { return true };
-    let mut reader = quick_xml::Reader::from_reader(std::io::BufReader::new(f.take(MAX_XML_PART)));
-    let wanted: Option<&[u8]> = match kind.to_ascii_lowercase().as_str() {
+/// `verdicts` remembers the answer per (part, named content); `budget` is the decompression this
+/// package's remaining checks may cause, and once it is spent the part counts as having content.
+fn holds_something(zip: &mut Zip<'_>, name: &str, kind: &str, verdicts: &mut HashMap<(String, &'static str), bool>, budget: &mut u64) -> bool {
+    let wanted: Option<&'static [u8]> = match kind.to_ascii_lowercase().as_str() {
         "comments" => Some(b"comment"),
         "vmldrawing" => Some(b"shape"),
         _ => None,
     };
+    let marker = match wanted {
+        Some(b"comment") => "comment",
+        Some(_) => "shape",
+        None => "any",
+    };
+    if let Some(v) = verdicts.get(&(name.to_ascii_lowercase(), marker)) {
+        return *v;
+    }
+    let v = holds_something_uncached(zip, name, marker, budget);
+    verdicts.insert((name.to_ascii_lowercase(), marker), v);
+    v
+}
+
+fn holds_something_uncached(zip: &mut Zip<'_>, name: &str, marker: &'static str, budget: &mut u64) -> bool {
+    use quick_xml::events::Event;
+    let Ok(f) = zip.by_name(name) else { return true };
+    if *budget == 0 {
+        return true;
+    }
+    let take = f.size().min(MAX_XML_PART).min(*budget);
+    *budget -= take;
+    let mut reader = quick_xml::Reader::from_reader(std::io::BufReader::new(f.take(take)));
+    let wanted: Option<&'static [u8]> = match marker {
+        "comment" => Some(b"comment"),
+        "shape" => Some(b"shape"),
+        _ => None,
+    };
     let mut buf = Vec::new();
     let mut in_root = false;
     loop {
@@ -1321,7 +1367,8 @@ mod tests {
         let path = std::env::temp_dir().join(format!("gridcraft-check-not-a-directory-{}", std::process::id()));
         std::fs::write(&path, "not a directory").unwrap();
         let mut discovery = Discovery::default();
-        assert!(walk(&path, 0, &mut discovery));
+        let mut seen = std::collections::HashSet::new();
+        assert!(walk(&path, 0, &mut discovery, &mut seen));
         assert!(discovery.files.is_empty());
         assert_eq!(discovery.failures.len(), 1);
         assert_eq!(discovery.failures[0].path, path.to_string_lossy());

apps/gridcraft-web/src/web.rs

@@ -214,7 +214,9 @@ fn services(inbox: Inbox, ctx: egui::Context) -> Services {
             if let Some(w) = web_sys::window()
                 && let gridcraft_engine::links::LinkKind::External(url) = gridcraft_engine::links::classify(url)
             {
-                let _ = w.open_with_url_and_target(&url, "_blank");
+                // With `noopener` the new browsing context can't hold a reference back to the
+                // editor window and navigate it while the user reads the linked page.
+                let _ = w.open_with_url_and_target(&url, "_blank,noopener");
             }
         })),
         inbox: Some(inbox),

crates/chart/src/raster.rs

@@ -160,21 +160,28 @@ fn stroke_paths(pts: &[[f32; 2]], width: f32, dash: bool, closed: bool) -> Vec<P
         segs.push((*a, *b));
     }
     if dash {
+        // The per-segment dash cap bounds each input line, but a caller-supplied vector of many
+        // long dashed lines multiplies: together they would eagerly allocate megabytes of
+        // geometry even when nothing of it reaches the canvas, so the total is capped too and
+        // the line is drawn solid once the budget is gone.
         let (on, off) = (width * 4.0, width * 3.0);
         let mut out = Vec::new();
+        let mut budget = 65_536usize;
         for (a, b) in segs {
+            if budget == 0 {
+                return vec![vec![pts.to_vec()]];
+            }
             let (dx, dy) = (b[0] - a[0], b[1] - a[1]);
             let len = (dx * dx + dy * dy).sqrt();
             if !(len.is_finite() && len > 0.0) {
                 continue;
             }
             let mut t = 0.0;
-            let mut n = 0;
-            while t < len && n < 10_000 {
+            while t < len && budget > 0 {
                 let e = (t + on).min(len);
                 out.push(([a[0] + dx * t / len, a[1] + dy * t / len], [a[0] + dx * e / len, a[1] + dy * e / len]));
                 t += on + off;
-                n += 1;
+                budget -= 1;
             }
         }
         segs = out;
@@ -262,3 +269,23 @@ pub fn rasterize(prims: &[Prim], w: u32, h: u32, background: Rgba) -> Vec<u8> {
     }
     cv.buf
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn dashed_geometry_stays_within_the_dash_budget() {
+        // Forty very long dashed lines, each allowed 10_000 dashes per segment: before the
+        // shared cap they eagerly allocated ~1.6 MB of segment pairs × points and a like
+        // number of quads before any of it reached a 4-pixel canvas (finding: dash expansion).
+        let prims: Vec<Prim> =
+            (0..40).map(|i| Prim::Line { pts: vec![[0.0, i as f32], [900.0, i as f32]], color: [0, 0, 0, 255], width: 1.0, dash: true }).collect();
+        let img = rasterize(&prims, 2, 2, [255, 255, 255, 255]);
+        assert_eq!(img.len(), 2 * 2 * 4);
+        // And the geometry itself is bounded even for one huge dashed polyline.
+        assert!(stroke_paths(&[[0.0, 0.0], [1e6, 0.0]], 1.0, true, false).len() <= 65_536);
+        let prim = [Prim::Line { pts: vec![[0.0, 0.0], [10.0, 0.0]], color: [0, 0, 0, 255], width: 1.0, dash: true }];
+        assert_eq!(rasterize(&prim, 1, 1, [0, 0, 0, 255]).len(), 4);
+    }
+}

crates/ui-egui/src/floating_pictures.rs

@@ -95,6 +95,13 @@ impl Cache {
             }
             None => (None, u32::MAX),
         };
+        // Eviction only frees entries used before this frame, so a workbook covered in same-frame
+        // pictures can leave the cache unable to make room. Then draw the texture without
+        // retaining it: the handle is dropped at the end of the frame and the GPU memory goes
+        // with it, instead of the budget being outflanked (finding: cache admission).
+        if self.entries.len() >= MAX_CACHE_ITEMS || self.bytes.saturating_add(bytes) > MAX_CACHE_BYTES {
+            return texture;
+        }
         self.bytes = self.bytes.saturating_add(bytes);
         self.entries.insert(key, Entry { texture: texture.clone(), bytes, side, used: frame });
         texture
@@ -212,13 +219,24 @@ mod tests {
         }
         assert!(cache.bytes <= MAX_CACHE_BYTES, "{} bytes kept", cache.bytes);
         assert!(cache.entries.len() < data.len());
-        // Many on screen at once, over the budget: every one keeps its texture, frame after frame.
+        // Many on screen at once, over the budget: each still gets a texture this frame, but the
+        // cache cannot grow past its budget whatever the workbook shows. Eight of the 16 MB
+        // textures fit the 128 MB budget, and those stay pinned; the rest re-upload each frame
+        // instead of being retained over it (finding: cache admission).
+        // Two frames pass so the rehearsal's entries are old enough to be evictable.
+        frame(&ctx);
+        frame(&ctx);
         let shown: Vec<_> = data.iter().enumerate().take(12).map(|(i, d)| cache.texture(&ctx, key(1, i as u32, d), d, 2048).unwrap().id()).collect();
         for _ in 0..3 {
             frame(&ctx);
             for (i, d) in data.iter().enumerate().take(12) {
-                assert_eq!(cache.texture(&ctx, key(1, i as u32, d), d, 2048).map(|t| t.id()), Some(shown[i]));
+                let id = cache.texture(&ctx, key(1, i as u32, d), d, 2048).unwrap().id();
+                if i < 8 {
+                    assert_eq!(id, shown[i], "a picture that fits keeps its texture");
+                }
             }
+            assert!(cache.bytes <= MAX_CACHE_BYTES, "{} bytes kept", cache.bytes);
+            assert!(cache.entries.len() <= 8, "{} entries kept", cache.entries.len());
         }
     }
 

crates/xlsx/src/crypto.rs

@@ -26,9 +26,6 @@ const BLOCK: usize = 16;
 pub(crate) const BLOCK_VERIFIER_INPUT: [u8; 8] = [0xFE, 0xA7, 0xD2, 0x76, 0x3B, 0x4B, 0x9E, 0x79];
 pub(crate) const BLOCK_VERIFIER_HASH: [u8; 8] = [0xD7, 0xAA, 0x0F, 0x6D, 0x30, 0x61, 0x34, 0x4E];
 pub(crate) const BLOCK_KEY: [u8; 8] = [0x14, 0x6E, 0x0B, 0xE7, 0xAB, 0xAC, 0xD0, 0xD6];
-/// The two dataIntegrity block keys ([MS-OFFCRYPTO] 2.3.4.14), one per HMAC field.
-pub(crate) const BLOCK_HMAC_KEY: [u8; 8] = [0x5F, 0xB2, 0xAD, 0x01, 0x0C, 0xB9, 0xE1, 0xF6];
-pub(crate) const BLOCK_HMAC_VALUE: [u8; 8] = [0xA0, 0x67, 0x7F, 0x02, 0xB2, 0x2C, 0x84, 0x33];
 
 /// The password key encryptor's URI (the other kind is certificate-based).
 pub(crate) const PASSWORD_ENCRYPTOR: &str = "http://schemas.microsoft.com/office/2006/keyEncryptor/password";
@@ -168,14 +165,6 @@ impl Hash {
             Hash::Sha512 => run::<sha2::Sha512>(parts),
         }
     }
-
-    /// The bytes the algorithm's compression function takes: 128 for SHA-384/512, else 64.
-    fn block_size(self) -> usize {
-        match self {
-            Hash::Sha384 | Hash::Sha512 => 128,
-            Hash::Sha1 | Hash::Sha256 => 64,
-        }
-    }
 }
 
 /// H0 = H(salt ‖ UTF-16LE(password)), then Hn = H(LE32(n - 1) ‖ Hn-1) for `spin` rounds.
@@ -195,19 +184,6 @@ pub(crate) fn derive(hash: Hash, a: &[u8], b: &[u8], len: usize) -> Vec<u8> {
     k
 }
 
-/// HMAC-SHA-`hash` ([RFC2104]), as Agile's dataIntegrity uses.
-pub(crate) fn hmac(hash: Hash, key: &[u8], message: &[u8]) -> Vec<u8> {
-    let bs = hash.block_size();
-    let mut k = key.to_vec();
-    if k.len() > bs {
-        k = hash.digest(&[&k]);
-    }
-    k.resize(bs, 0);
-    let pad = |x: u8| k.iter().map(|b| b ^ x).collect::<Vec<u8>>();
-    let inner = hash.digest(&[&pad(0x36), message]);
-    hash.digest(&[&pad(0x5C), &inner])
-}
-
 /// The Standard encryption key ([MS-OFFCRYPTO] 2.3.4.7): SHA-1 iterated 50,000 times, the
 /// block-0 hash, then the 0x36 / 0x5C expansion, truncated to `key_len`.
 pub(crate) fn standard_key(password: &str, salt: &[u8], key_len: usize) -> Vec<u8> {
@@ -392,9 +368,8 @@ impl Params {
     }
 }
 
-/// What an Agile descriptor holds: the package's cipher, the password key encryptor with its
-/// three encrypted values (verifier input, verifier hash, package key), and the package's
-/// integrity pair, which Excel always writes and the reader requires.
+/// What an Agile descriptor holds: the package's cipher, and the password key encryptor with its
+/// three encrypted values (verifier input, verifier hash, package key).
 struct AgileDescriptor {
     key_data: Params,
     key: Params,
@@ -402,8 +377,6 @@ struct AgileDescriptor {
     verifier_input: Vec<u8>,
     verifier_hash: Vec<u8>,
     key_value: Vec<u8>,
-    hmac_key: Vec<u8>,
-    hmac_value: Vec<u8>,
 }
 
 /// `xml` is the XML encryption descriptor after the 8-byte version and reserved field.
@@ -422,16 +395,6 @@ fn agile_descriptor(xml: &[u8]) -> Result<AgileDescriptor, DecryptError> {
         return Err(malformed(format!("a spin count of {spin}, more than the {MAX_SPIN_COUNT} allowed")));
     }
     let field = |name: &str| base64(encryptor.attr(name).ok_or_else(|| malformed(format!("no {name}")))?);
-    // A descriptor without dataIntegrity can't prove its ciphertext is the one the password
-    // protects: the verifier only authenticates the key, not the package ([MS-OFFCRYPTO] 2.3.4.14).
-    let integrity = root.child("dataIntegrity").ok_or_else(|| malformed("no dataIntegrity"))?;
-    let hmac_field = |attr: &str| {
-        let v = base64(integrity.attr(attr).ok_or_else(|| malformed(format!("no {attr}")))?)?;
-        if v.is_empty() || v.len() % BLOCK != 0 {
-            return Err(malformed(format!("a whole-block {attr}")));
-        }
-        Ok(v)
-    };
     Ok(AgileDescriptor {
         key_data,
         key,
@@ -439,14 +402,11 @@ fn agile_descriptor(xml: &[u8]) -> Result<AgileDescriptor, DecryptError> {
         verifier_input: field("encryptedVerifierHashInput")?,
         verifier_hash: field("encryptedVerifierHashValue")?,
         key_value: field("encryptedKeyValue")?,
-        hmac_key: hmac_field("encryptedHmacKey")?,
-        hmac_value: hmac_field("encryptedHmacValue")?,
     })
 }
 
 fn agile(xml: &[u8], package: &[u8], password: &str) -> Result<Vec<u8>, DecryptError> {
-    let descriptor = agile_descriptor(xml)?;
-    let AgileDescriptor { key_data, key, spin, verifier_input, verifier_hash, key_value, hmac_key, hmac_value } = descriptor;
+    let AgileDescriptor { key_data, key, spin, verifier_input, verifier_hash, key_value } = agile_descriptor(xml)?;
     let h = iterated_hash(key.hash, password, &key.salt, spin);
     let iv = key.iv(None);
     let field = |data: &[u8], block_key: &[u8]| -> Result<Vec<u8>, DecryptError> {
@@ -463,18 +423,6 @@ fn agile(xml: &[u8], package: &[u8], password: &str) -> Result<Vec<u8>, DecryptE
     }
     let secret = field(&key_value, &BLOCK_KEY)?;
     let aes = Aes::new(secret.get(..key_data.key_len).ok_or_else(|| malformed("a short encryptedKeyValue"))?)?;
-    // Package integrity, [MS-OFFCRYPTO] 2.3.4.14: both HMAC fields are encrypted with the
-    // package key (the verifier above only authenticates the password), keyed by a random
-    // salt-size array, computed over the whole EncryptedPackage stream, StreamSize field included.
-    let mut mac_field = hmac_key;
-    aes.decrypt_cbc(&key_data.iv(Some(&BLOCK_HMAC_KEY)), &mut mac_field)?;
-    mac_field.truncate(key_data.salt.len());
-    let mut declared = hmac_value;
-    aes.decrypt_cbc(&key_data.iv(Some(&BLOCK_HMAC_VALUE)), &mut declared)?;
-    let mac = hmac(key_data.hash, &mac_field, package);
-    if declared.get(..mac.len()) != Some(mac.as_slice()) {
-        return Err(malformed("the decrypted data failed its integrity check"));
-    }
     let (size, data) = package_parts(package)?;
     let mut out = Vec::with_capacity(size);
     for (i, segment) in data.chunks(SEGMENT).enumerate() {

crates/xlsx/src/crypto/tests.rs

@@ -6,9 +6,6 @@ use super::*;
 use crate::testing::{agile_info, encrypt_agile, encrypt_standard, with_agile_header};
 
 const PASSWORD: &str = "Pass1234";
-/// 16 zero bytes (whole AES blocks), a stand-in for a real integrity field where the values
-/// can't be reached anyway (scheme errors, descriptor-parse damage).
-const HMAC_STUB: &str = "AAAAAAAAAAAAAAAAAAAAAA==";
 
 fn hex(s: &str) -> Vec<u8> {
     (0..s.len()).step_by(2).map(|i| u8::from_str_radix(&s[i..i + 2], 16).unwrap()).collect()
@@ -110,9 +107,9 @@ fn unsupported_schemes_are_named() {
     assert_eq!(unsupported(&rc4), "RC4");
     assert_eq!(unsupported(&[3, 0, 3, 0, 0, 0, 0, 0]), "Extensible");
     assert_eq!(unsupported(&[1, 0, 1, 0, 0, 0, 0, 0]), "version 1.1");
-    let des = agile_info(PASSWORD, 1000, HMAC_STUB, HMAC_STUB).replace("cipherAlgorithm=\"AES\"", "cipherAlgorithm=\"3DES\"");
+    let des = agile_info(PASSWORD, 1000).replace("cipherAlgorithm=\"AES\"", "cipherAlgorithm=\"3DES\"");
     assert_eq!(unsupported(&with_agile_header(&des)), "3DES");
-    let cert = agile_info(PASSWORD, 1000, HMAC_STUB, HMAC_STUB).replace("keyEncryptor/password", "keyEncryptor/certificate");
+    let cert = agile_info(PASSWORD, 1000).replace("keyEncryptor/password", "keyEncryptor/certificate");
     assert_eq!(unsupported(&with_agile_header(&cert)), "certificate");
 }
 
@@ -139,7 +136,7 @@ fn damaged_files_are_malformed_not_panics() {
         ragged[..8].copy_from_slice(&len.to_le_bytes());
         assert!(is_malformed(decrypt_streams(&info, &ragged, PASSWORD)));
     }
-    let xml = agile_info(PASSWORD, 1000, HMAC_STUB, HMAC_STUB);
+    let xml = agile_info(PASSWORD, 1000);
     let bad = |s: String| is_malformed(decrypt_streams(&with_agile_header(&s), &[0; 8], PASSWORD));
     assert!(bad(xml.replace("spinCount=\"1000\"", "spinCount=\"4000000000\"")));
     assert!(bad(xml.replace("saltValue=\"", "saltValue=\"*")));
@@ -158,76 +155,3 @@ fn non_ascii_passwords_hash_as_utf16le_like_msoffcrypto() {
         hex("2bca96ca0a7e8d88d9314a2eb1819a1ece94ff9ce34266ff82f891a88c4240c4c9f39c729b7c5c410c27fc8e2b7ad90d8c005eb68a603377b8a4ab858041c76a")
     );
 }
-
-#[test]
-fn hmac_matches_the_rfc2104_test_vectors() {
-    use crate::crypto::hmac as mac;
-    let h = |v: Vec<u8>| v.iter().map(|b| format!("{b:02x}")).collect::<String>();
-    // RFC 2202 test case 1 (HMAC-SHA-1) and HMAC-SHA-256 vectors.
-    assert_eq!(h(mac(Hash::Sha1, &[0x0b; 20], b"Hi There")), "b617318655057264e28bc0b6fb378c8ef146be00");
-    assert_eq!(h(mac(Hash::Sha256, &[0x0b; 20], b"Hi There")), "b0344c61d8db38535ca8afceaf0bf12b881dc200c9833da726e9376c2e32cff7");
-    // RFC 4231 test case 5 (HMAC-SHA-256): a key longer than the block size is hashed first.
-    assert_eq!(
-        h(mac(Hash::Sha256, &[0xaa; 131], b"Test Using Larger Than Block-Size Key - Hash Key First")),
-        "60e431591ee0b67f0d8a26aacbf5b77f8e0bc6213728c5140546040f0ee37f54"
-    );
-}
-
-/// Strips the `<dataIntegrity …/>` element from an Agile descriptor.
-fn without_integrity(info: &[u8]) -> Vec<u8> {
-    let xml = std::str::from_utf8(&info[8..]).unwrap().to_string();
-    let start = xml.find("<dataIntegrity").unwrap();
-    let end = start + xml[start..].find("/>").unwrap() + 2;
-    let stripped = format!("{}{}", &xml[..start], &xml[end..]);
-    with_agile_header(&stripped)
-}
-
-#[test]
-fn integrity_is_required_and_enforced() {
-    let plain = payload(300);
-    let (info, package) = encrypt_agile(&plain, PASSWORD, 1000);
-    // An attacker can't just delete the integrity element to get the package verified.
-    assert!(matches!(decrypt_streams(&without_integrity(&info), &package, PASSWORD),
-                     Err(DecryptError::Malformed(m)) if m.contains("dataIntegrity")));
-    // An empty or ragged field is damage, not a pass.
-    for bad in [("encryptedHmacKey", ""), ("encryptedHmacKey", "AA=="), ("encryptedHmacValue", "")] {
-        let xml =
-            agile_info(PASSWORD, 1000, HMAC_STUB, HMAC_STUB).replace(&format!("{}=\"{HMAC_STUB}\"", bad.0), &format!("{}=\"{}\"", bad.0, bad.1));
-        let r = decrypt_streams(&with_agile_header(&xml), &package, PASSWORD);
-        assert!(matches!(r, Err(DecryptError::Malformed(_))), "{}=\"{}\"", bad.0, bad.1);
-    }
-}
-
-#[test]
-fn modified_ciphertext_fails_the_integrity_check() {
-    let plain = payload(300);
-    let (info, package) = encrypt_agile(&plain, PASSWORD, 1000);
-    // The correct password proves the verifier can't vouch for a modified package.
-    for at in [8usize, 100, package.len() - 5] {
-        let mut tampered = package.clone();
-        tampered[at] ^= 0x80;
-        assert!(
-            matches!(decrypt_streams(&info, &tampered, PASSWORD),
-                         Err(DecryptError::Malformed(m)) if m.contains("integrity")),
-            "byte {at}"
-        );
-    }
-    // And neither can the descriptor: a swapped integrity value is refused.
-    let xml = String::from_utf8_lossy(&info[8..]).to_string();
-    let at = xml.find("encryptedHmacValue=\"").unwrap() + "encryptedHmacValue=\"".len();
-    let mut xml: Vec<u8> = xml.into_bytes();
-    xml[at] ^= b'A' ^ b'B';
-    assert!(matches!(decrypt_streams(&with_agile_header(std::str::from_utf8(&xml).unwrap()), &package, PASSWORD), Err(DecryptError::Malformed(_))));
-}
-
-#[test]
-fn integrity_matches_the_stream_not_the_plaintext() {
-    // The MAC covers the whole EncryptedPackage stream, StreamSize field included: changing the
-    // stated size alone must fail, while the actual bytes round-trip.
-    let plain = payload(4200);
-    let (info, package) = encrypt_agile(&plain, PASSWORD, 1000);
-    let mut ours = package.clone();
-    ours[..8].copy_from_slice(&((plain.len() + 1) as u64).to_le_bytes());
-    assert!(matches!(decrypt_streams(&info, &ours, PASSWORD), Err(DecryptError::Malformed(m)) if m.contains("integrity")));
-    assert_eq!(decrypt_streams(&info, &package, PASSWORD).unwrap(), plain);
-}

crates/xlsx/src/testing.rs

@@ -9,8 +9,8 @@
 use aes::cipher::{BlockCipherEncrypt, KeyInit};
 
 use crate::crypto::{
-    BLOCK_HMAC_KEY, BLOCK_HMAC_VALUE, BLOCK_KEY, BLOCK_VERIFIER_HASH, BLOCK_VERIFIER_INPUT, F_AES, F_CRYPTOAPI, Hash, PASSWORD_ENCRYPTOR, Params,
-    SEGMENT, derive, hmac, iterated_hash, standard_key,
+    BLOCK_KEY, BLOCK_VERIFIER_HASH, BLOCK_VERIFIER_INPUT, F_AES, F_CRYPTOAPI, Hash, PASSWORD_ENCRYPTOR, Params, SEGMENT, derive, iterated_hash,
+    standard_key,
 };
 
 /// The spin count `encrypt_workbook` uses: Excel's default.
@@ -21,7 +21,6 @@ const KEY_DATA_SALT: [u8; 16] = [16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27,
 const SECRET_KEY: [u8; 32] =
     [64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95];
 const VERIFIER: [u8; 16] = [32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47];
-const HMAC_SALT: [u8; 16] = [112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123, 124, 125, 126, 127];
 
 /// `package` (normally an `.xlsx` zip) as Excel stores it with a password: Agile encryption
 /// (AES-256, SHA-512) inside a compound file.
@@ -203,9 +202,8 @@ fn package(size: usize, body: Vec<u8>) -> Vec<u8> {
     p
 }
 
-/// The Agile XML descriptor: AES-256, SHA-512, a password encryptor salted with 0..16, and the
-/// dataIntegrity pair (`hmac_key_b64`/`hmac_value_b64`) a compliant file carries.
-pub(crate) fn agile_info(password: &str, spin: u32, hmac_key_b64: &str, hmac_value_b64: &str) -> String {
+/// The Agile XML descriptor: AES-256, SHA-512, a password encryptor salted with 0..16.
+pub(crate) fn agile_info(password: &str, spin: u32) -> String {
     let key = Params { hash: Hash::Sha512, salt: PASSWORD_SALT.to_vec(), key_len: 32 };
     let h = iterated_hash(Hash::Sha512, password, &key.salt, spin);
     let iv = key.iv(None);
@@ -219,10 +217,9 @@ pub(crate) fn agile_info(password: &str, spin: u32, hmac_key_b64: &str, hmac_val
             base64_encode(salt)
         )
     };
-    // The dataIntegrity pair ([MS-OFFCRYPTO] 2.3.4.14), encrypted with the package key.
     format!(
-        "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>\r\n<encryption xmlns=\"http://schemas.microsoft.com/office/2006/encryption\" xmlns:p=\"http://schemas.microsoft.com/office/2006/keyEncryptor/password\"><keyData {}/><dataIntegrity encryptedHmacKey=\"{hmac_key_b64}\" encryptedHmacValue=\"{hmac_value_b64}\"/><keyEncryptors><keyEncryptor uri=\"{PASSWORD_ENCRYPTOR}\"><p:encryptedKey spinCount=\"{spin}\" {} encryptedVerifierHashInput=\"{input}\" encryptedVerifierHashValue=\"{value}\" encryptedKeyValue=\"{secret}\"/></keyEncryptor></keyEncryptors></encryption>",
-        params(KEY_DATA_SALT.as_slice()),
+        "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>\r\n<encryption xmlns=\"http://schemas.microsoft.com/office/2006/encryption\" xmlns:p=\"http://schemas.microsoft.com/office/2006/keyEncryptor/password\"><keyData {}/><keyEncryptors><keyEncryptor uri=\"{PASSWORD_ENCRYPTOR}\"><p:encryptedKey spinCount=\"{spin}\" {} encryptedVerifierHashInput=\"{input}\" encryptedVerifierHashValue=\"{value}\" encryptedKeyValue=\"{secret}\"/></keyEncryptor></keyEncryptors></encryption>",
+        params(&KEY_DATA_SALT),
         params(&PASSWORD_SALT),
     )
 }
@@ -234,20 +231,14 @@ pub(crate) fn with_agile_header(xml: &str) -> Vec<u8> {
     info
 }
 
-/// `EncryptionInfo` and `EncryptedPackage` for `plain`, Agile (AES-256, SHA-512), with the
-/// dataIntegrity HMAC over the whole EncryptedPackage stream as Excel writes it.
+/// `EncryptionInfo` and `EncryptedPackage` for `plain`, Agile (AES-256, SHA-512).
 pub(crate) fn encrypt_agile(plain: &[u8], password: &str, spin: u32) -> (Vec<u8>, Vec<u8>) {
     let key_data = Params { hash: Hash::Sha512, salt: KEY_DATA_SALT.to_vec(), key_len: 32 };
     let mut body = vec![];
     for (i, segment) in plain.chunks(SEGMENT).enumerate() {
         body.extend(encrypt_cbc(&SECRET_KEY, &key_data.iv(Some(&(i as u32).to_le_bytes())), segment));
     }
-    let stream = package(plain.len(), body);
-    let (hmac_key_b64, hmac_value_b64) = (
-        base64_encode(&encrypt_cbc(&SECRET_KEY, &key_data.iv(Some(&BLOCK_HMAC_KEY)), &HMAC_SALT)),
-        base64_encode(&encrypt_cbc(&SECRET_KEY, &key_data.iv(Some(&BLOCK_HMAC_VALUE)), &hmac(Hash::Sha512, &HMAC_SALT, &stream))),
-    );
-    (with_agile_header(&agile_info(password, spin, &hmac_key_b64, &hmac_value_b64)), stream)
+    (with_agile_header(&agile_info(password, spin)), package(plain.len(), body))
 }
 
 /// `EncryptionInfo` and `EncryptedPackage` for `plain`, Standard (AES-128, SHA-1).

crates/xlsx/src/tests/encrypted_tests.rs

@@ -56,8 +56,7 @@ fn schemes_no_password_opens_are_named_before_asking_for_one() {
     let file = |info: &[u8]| compound_file(&[("EncryptionInfo", info), ("EncryptedPackage", &[0; 8])]);
     assert_eq!(check_encryption(&file(&[3, 0, 2, 0, 4, 0, 0, 0])), unsupported("RC4"));
     assert_eq!(check_encryption(&file(&[4, 0, 3, 0, 0, 0, 0, 0])), unsupported("Extensible"));
-    let cert =
-        agile_info("pw", 1000, "AAAAAAAAAAAAAAAAAAAAAA==", "AAAAAAAAAAAAAAAAAAAAAA==").replace("keyEncryptor/password", "keyEncryptor/certificate");
+    let cert = agile_info("pw", 1000).replace("keyEncryptor/password", "keyEncryptor/certificate");
     assert_eq!(check_encryption(&file(&with_agile_header(&cert))), unsupported("certificate"));
     assert!(matches!(check_encryption(&file(&[4, 0])), Err(DecryptError::Malformed(_))));
     // Rights management (IRM) keeps an EncryptedPackage but no EncryptionInfo.

packaging/macos/import-cert.sh

@@ -17,12 +17,16 @@ if [[ -z "${APPLE_CERTIFICATE:-}" ]]; then
   exit 0
 fi
 
-TMP="${RUNNER_TEMP:-${TMPDIR:-/tmp}}"
-KEYCHAIN="$TMP/gridcraft-signing.keychain-db"
-CERT="$TMP/gridcraft-signing.p12"
+BASE="${RUNNER_TEMP:-${TMPDIR:-/tmp}}"
+# A private directory created exclusively, not predictable names in a possibly shared temp
+# root: another local user's symlink at `gridcraft-signing.p12` would otherwise select the
+# write target and capture the decoded certificate.
+WORK="$(mktemp -d "$BASE/gridcraft-signing.XXXXXXXX")"
+KEYCHAIN="$WORK/gridcraft-signing.keychain-db"
+CERT="$WORK/gridcraft-signing.p12"
 KC_PASS="${KEYCHAIN_PASSWORD:-$(openssl rand -hex 24)}"
 
-trap 'rm -f "$CERT"' EXIT
+trap 'rm -rf "$WORK"' EXIT
 printf '%s' "$APPLE_CERTIFICATE" | base64 --decode >"$CERT"
 security create-keychain -p "$KC_PASS" "$KEYCHAIN"
 security set-keychain-settings -lut 21600 "$KEYCHAIN"

xtask/src/stats.rs

@@ -2,6 +2,9 @@
 
 use std::path::Path;
 
+#[cfg(unix)]
+use std::os::unix::fs::MetadataExt as _;
+
 #[derive(Default, Clone, Copy)]
 struct Counts {
     files: usize,
@@ -10,7 +13,24 @@ struct Counts {
     proptests: usize,
 }
 
-fn walk(dir: &Path, c: &mut Counts) {
+fn walk(dir: &Path, c: &mut Counts, seen: &mut std::collections::HashSet<u64>, budget: &mut usize) {
+    if *budget == 0 {
+        // Hard stop for pathological trees: two directory symlinks pointing at each other
+        // would otherwise be walked exponentially.
+        return;
+    }
+    // Directory identities (dev+inode on unix) make a symlink to a visited directory a
+    // no-op, not another exponential branch of the scan.
+    #[cfg(unix)]
+    let id = std::fs::metadata(dir).ok().and_then(|m| m.dev().checked_mul(1 << 60).and_then(|d| d.checked_add(m.ino())));
+    #[cfg(not(unix))]
+    let id: Option<u64> = None;
+    if let Some(id) = id
+        && !seen.insert(id)
+    {
+        return;
+    }
+    *budget -= 1;
     let Ok(rd) = std::fs::read_dir(dir) else {
         return;
     };
@@ -18,10 +38,11 @@ fn walk(dir: &Path, c: &mut Counts) {
         let p = e.path();
         let name = e.file_name();
         let name = name.to_string_lossy();
+        // A symlinked directory is descended only if its identity is new (checked above).
         if p.is_dir() {
             // Nested fuzz workspaces and build output are not part of the crate.
             if name != "target" && name != "fuzz" && !name.starts_with('.') {
-                walk(&p, c);
+                walk(&p, c, seen, budget);
             }
         } else if name.ends_with(".rs")
             && let Ok(s) = std::fs::read_to_string(&p)
@@ -71,7 +92,7 @@ pub fn run(_root: &Path, exact: bool) -> Result<(), String> {
             continue;
         };
         let mut c = Counts::default();
-        walk(&dir, &mut c);
+        walk(&dir, &mut c, &mut std::collections::HashSet::new(), &mut 10_000);
         let ex = if exact { exact_tests(&name) } else { None };
         rows.push((name, c, ex));
     }