From f9ba8ecaf50f01fe0ec4c6caaf3d3b19122f9fee Mon Sep 17 00:00:00 2001 From: Loggableim <160679982+Loggableim@users.noreply.github.com> Date: Sun, 19 Jul 2026 01:12:00 +0200 Subject: [PATCH 1/5] harden: validate certificate table bounds in pe_hash --- ssign-core/src/authenticode.rs | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/ssign-core/src/authenticode.rs b/ssign-core/src/authenticode.rs index 041ee0b..7bc7da0 100644 --- a/ssign-core/src/authenticode.rs +++ b/ssign-core/src/authenticode.rs @@ -82,10 +82,17 @@ fn pe_layout(pe: &[u8]) -> Result { /// Compute the Authenticode SHA-256 hash of a PE image. pub fn pe_hash(pe: &[u8]) -> Result<[u8; 32]> { let l = pe_layout(pe)?; + let cert_end = l + .cert_table_off + .checked_add(l.cert_table_size) + .context("certificate table length overflows")?; + if l.cert_table_off > pe.len() || cert_end > pe.len() { + bail!("certificate table lies outside the PE file"); + } let (cert_start, cert_end) = if l.cert_table_size == 0 { (pe.len(), pe.len()) } else { - (l.cert_table_off, l.cert_table_off + l.cert_table_size) + (l.cert_table_off, cert_end) }; let mut h = Sha256::new(); h.update(&pe[..l.checksum_off]); @@ -414,6 +421,18 @@ mod tests { assert_eq!(pe_hash(&odd).unwrap(), pe_hash(&padded).unwrap()); } + #[test] + fn rejects_certificate_table_outside_the_file() { + let mut pe = vec![0u8; 0x200]; + pe[..2].copy_from_slice(b"MZ"); + pe[0x3c..0x40].copy_from_slice(&(0x80u32).to_le_bytes()); + pe[0x80..0x84].copy_from_slice(b"PE\0\0"); + pe[0x98..0x9a].copy_from_slice(&[0x0b, 0x01]); + pe[0x118..0x11c].copy_from_slice(&(0xffff_fff0u32).to_le_bytes()); + pe[0x11c..0x120].copy_from_slice(&(0x20u32).to_le_bytes()); + assert!(pe_hash(&pe).is_err()); + } + #[test] fn pe_hash_matches_osslsigncode() { let pe = include_bytes!("../tests/fixtures/hello.exe"); From f7779f7549f4b6ce539c4fcd0c82da15f85e3b97 Mon Sep 17 00:00:00 2001 From: Loggableim <160679982+Loggableim@users.noreply.github.com> Date: Sun, 19 Jul 2026 01:17:36 +0200 Subject: [PATCH 2/5] harden: validate CMS structure of timestamp token --- ssign-core/src/timestamp.rs | 38 +++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/ssign-core/src/timestamp.rs b/ssign-core/src/timestamp.rs index 2957af1..8d5fccb 100644 --- a/ssign-core/src/timestamp.rs +++ b/ssign-core/src/timestamp.rs @@ -7,6 +7,7 @@ use sha2::{Digest, Sha256}; use std::time::Duration; const OID_SHA256: &str = "2.16.840.1.101.3.4.2.1"; +const OID_PKCS7_SIGNED_DATA: &str = "1.2.840.113549.1.7.2"; /// Request a timestamp over `signature` from the RFC3161 TSA at `url`; returns /// the DER `TimeStampToken` (a PKCS#7 ContentInfo). @@ -54,5 +55,42 @@ pub fn fetch(url: &str, signature: &[u8]) -> Result> { bail!("TSA rejected the request (status {status})"); } let token = top.get(1).context("TSR has no timeStampToken")?; + validate_timestamp_token(token)?; Ok(token.to_vec()) } + +fn validate_timestamp_token(token: &[u8]) -> Result<()> { + if token.first() != Some(&0x30) { + bail!("timeStampToken is not a DER SEQUENCE"); + } + let children = asn1::children(token).map_err(|e| anyhow::anyhow!("bad timeStampToken: {e}"))?; + if children.len() != 2 || children[0] != asn1::oid(OID_PKCS7_SIGNED_DATA).as_slice() { + bail!("timeStampToken is not CMS SignedData"); + } + let wrapped = children[1]; + if wrapped.first() != Some(&0xa0) { + bail!("CMS SignedData is missing its explicit wrapper"); + } + let signed_data = asn1::children(wrapped) + .map_err(|e| anyhow::anyhow!("bad CMS SignedData wrapper: {e}"))?; + if signed_data.len() != 1 || signed_data[0].first() != Some(&0x30) { + bail!("CMS SignedData wrapper did not contain SignedData"); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn accepts_only_cms_signed_data_tokens() { + let signed_data = asn1::seq(&[]); + let token = asn1::seq(&[ + &asn1::oid("1.2.840.113549.1.7.2"), + &asn1::ctx(0, &signed_data), + ]); + assert!(validate_timestamp_token(&token).is_ok()); + assert!(validate_timestamp_token(&asn1::seq(&[])).is_err()); + } +} From 457cb87f9a665a81a1b69d13b40cf6ab3eed655f Mon Sep 17 00:00:00 2001 From: Loggableim <160679982+Loggableim@users.noreply.github.com> Date: Sun, 19 Jul 2026 01:22:33 +0200 Subject: [PATCH 3/5] style: format timestamp token validator --- ssign-core/src/timestamp.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ssign-core/src/timestamp.rs b/ssign-core/src/timestamp.rs index 8d5fccb..801c967 100644 --- a/ssign-core/src/timestamp.rs +++ b/ssign-core/src/timestamp.rs @@ -71,8 +71,8 @@ fn validate_timestamp_token(token: &[u8]) -> Result<()> { if wrapped.first() != Some(&0xa0) { bail!("CMS SignedData is missing its explicit wrapper"); } - let signed_data = asn1::children(wrapped) - .map_err(|e| anyhow::anyhow!("bad CMS SignedData wrapper: {e}"))?; + let signed_data = + asn1::children(wrapped).map_err(|e| anyhow::anyhow!("bad CMS SignedData wrapper: {e}"))?; if signed_data.len() != 1 || signed_data[0].first() != Some(&0x30) { bail!("CMS SignedData wrapper did not contain SignedData"); } From cb40d749d02544291b2057eeac5197608593e5f8 Mon Sep 17 00:00:00 2001 From: Loggableim <160679982+Loggableim@users.noreply.github.com> Date: Sun, 19 Jul 2026 01:30:00 +0200 Subject: [PATCH 4/5] harden: atomically publish signed output and backups --- src/main.rs | 188 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 182 insertions(+), 6 deletions(-) diff --git a/src/main.rs b/src/main.rs index 993a927..cd734a5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,7 +8,9 @@ use ssign_core::{auth, authenticode, card, client, otp, sign, timestamp}; use anyhow::{bail, Context, Result}; use clap::Parser; -use std::path::PathBuf; +use std::fs::{self, File, OpenOptions}; +use std::io::{self, Write}; +use std::path::{Path, PathBuf}; use std::time::{SystemTime, UNIX_EPOCH}; /// Authenticode-sign Windows binaries (exe/dll/msi/sys) with a Certum SimplySign @@ -162,18 +164,161 @@ fn run(cli: &Cli, otp: Otp) -> Result<()> { .with_context(|| format!("assembling signature for {}", file.display()))?; let out = output_path(file, cli.output_dir.as_deref())?; - if cli.backup && out == *file { - std::fs::write(file.with_extension("orig"), &pe).context("writing backup")?; - } - std::fs::write(&out, &signed).with_context(|| format!("writing {}", out.display()))?; + write_signed_file(file, &out, &pe, &signed, cli.backup) + .with_context(|| format!("writing {}", out.display()))?; println!("signed {}", out.display()); } Ok(()) } +fn write_signed_file( + input: &Path, + out: &Path, + original: &[u8], + signed: &[u8], + backup: bool, +) -> Result<()> { + let signed_temp = write_synced_temp(out, "signed", signed)?; + let result = (|| -> Result<()> { + match fs::metadata(out) { + Ok(metadata) => fs::set_permissions(&signed_temp, metadata.permissions()) + .with_context(|| format!("preserving permissions for {}", out.display()))?, + Err(err) if err.kind() == io::ErrorKind::NotFound => {} + Err(err) => { + return Err(err).with_context(|| format!("reading metadata for {}", out.display())) + } + } + + if backup && input == out { + publish_backup(input, out, original)?; + } + + atomic_replace(&signed_temp, out) + .with_context(|| format!("atomically replacing {}", out.display()))?; + Ok(()) + })(); + + if result.is_err() { + let _ = fs::remove_file(&signed_temp); + } + result +} + +fn publish_backup(input: &Path, out: &Path, original: &[u8]) -> Result<()> { + let backup_path = backup_path(input)?; + let backup_temp = write_synced_temp(out, "backup", original)?; + let result = fs::hard_link(&backup_temp, &backup_path) + .with_context(|| format!("creating backup {}", backup_path.display())); + let _ = fs::remove_file(&backup_temp); + result +} + +fn backup_path(input: &Path) -> Result { + let mut backup_name = input + .file_name() + .context("input has no file name")? + .to_os_string(); + backup_name.push(".orig"); + Ok(input.with_file_name(backup_name)) +} + +fn write_synced_temp(out: &Path, purpose: &str, contents: &[u8]) -> Result { + let (mut temp, temp_path) = create_temp_file(out, purpose)?; + let write_result = (|| -> io::Result<()> { + temp.write_all(contents)?; + temp.sync_all() + })(); + drop(temp); + + if let Err(err) = write_result { + let _ = fs::remove_file(&temp_path); + return Err(err).with_context(|| format!("writing temporary file {}", temp_path.display())); + } + + Ok(temp_path) +} + +fn create_temp_file(out: &Path, purpose: &str) -> Result<(File, PathBuf)> { + const MAX_ATTEMPTS: u32 = 32; + + let directory = out + .parent() + .filter(|path| !path.as_os_str().is_empty()) + .unwrap_or_else(|| Path::new(".")); + let file_name = out.file_name().context("output has no file name")?; + let nonce = SystemTime::now() + .duration_since(UNIX_EPOCH) + .context("system clock before 1970")? + .as_nanos(); + + for attempt in 0..MAX_ATTEMPTS { + let mut temp_name = file_name.to_os_string(); + temp_name.push(format!( + ".{purpose}.{}.{}.{}.tmp", + std::process::id(), + nonce, + attempt + )); + let temp_path = directory.join(temp_name); + match OpenOptions::new() + .write(true) + .create_new(true) + .open(&temp_path) + { + Ok(file) => return Ok((file, temp_path)), + Err(err) if err.kind() == io::ErrorKind::AlreadyExists => continue, + Err(err) => { + return Err(err) + .with_context(|| format!("creating temporary file {}", temp_path.display())) + } + } + } + + bail!( + "could not create a unique temporary file for {}", + out.display() + ) +} + +#[cfg(windows)] +fn atomic_replace(temp: &Path, out: &Path) -> io::Result<()> { + use std::os::windows::ffi::OsStrExt; + + const MOVEFILE_REPLACE_EXISTING: u32 = 0x0000_0001; + const MOVEFILE_WRITE_THROUGH: u32 = 0x0000_0008; + + unsafe extern "system" { + fn MoveFileExW( + existing_file_name: *const u16, + new_file_name: *const u16, + flags: u32, + ) -> i32; + } + + let temp_wide: Vec = temp.as_os_str().encode_wide().chain(Some(0)).collect(); + let out_wide: Vec = out.as_os_str().encode_wide().chain(Some(0)).collect(); + if unsafe { + MoveFileExW( + temp_wide.as_ptr(), + out_wide.as_ptr(), + MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH, + ) + } == 0 + { + Err(io::Error::last_os_error()) + } else { + Ok(()) + } +} + +#[cfg(not(windows))] +fn atomic_replace(temp: &Path, out: &Path) -> io::Result<()> { + fs::rename(temp, out) +} + /// Where a signed file is written: into `output_dir` (same file name) if given, /// otherwise in place. -fn output_path(file: &std::path::Path, output_dir: Option<&std::path::Path>) -> Result { +fn output_path(file: &Path, output_dir: Option<&Path>) -> Result { match output_dir { None => Ok(file.to_path_buf()), Some(dir) => { @@ -183,3 +328,34 @@ fn output_path(file: &std::path::Path, output_dir: Option<&std::path::Path>) -> } } } + +#[cfg(test)] +mod tests { + use super::*; + + fn unique_test_dir(label: &str) -> PathBuf { + let nonce = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos(); + let dir = + std::env::temp_dir().join(format!("ssign-{label}-{}-{nonce}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + dir + } + + #[test] + fn in_place_backup_is_preserved_and_never_overwritten() { + let dir = unique_test_dir("backup"); + let input = dir.join("app.exe"); + std::fs::write(&input, b"original").unwrap(); + write_signed_file(&input, &input, b"original", b"signed", true).unwrap(); + assert_eq!(std::fs::read(&input).unwrap(), b"signed"); + assert_eq!( + std::fs::read(dir.join("app.exe.orig")).unwrap(), + b"original" + ); + assert!(write_signed_file(&input, &input, b"signed", b"new", true).is_err()); + std::fs::remove_dir_all(dir).unwrap(); + } +} From d7c42d22a84f2c8b119d83898d1013dc21274f32 Mon Sep 17 00:00:00 2001 From: Loggableim <160679982+Loggableim@users.noreply.github.com> Date: Wed, 22 Jul 2026 08:27:24 +0200 Subject: [PATCH 5/5] fix(authenticode): allow stale offset for empty certificate table --- ssign-core/src/authenticode.rs | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/ssign-core/src/authenticode.rs b/ssign-core/src/authenticode.rs index 7bc7da0..c94127d 100644 --- a/ssign-core/src/authenticode.rs +++ b/ssign-core/src/authenticode.rs @@ -82,16 +82,16 @@ fn pe_layout(pe: &[u8]) -> Result { /// Compute the Authenticode SHA-256 hash of a PE image. pub fn pe_hash(pe: &[u8]) -> Result<[u8; 32]> { let l = pe_layout(pe)?; - let cert_end = l - .cert_table_off - .checked_add(l.cert_table_size) - .context("certificate table length overflows")?; - if l.cert_table_off > pe.len() || cert_end > pe.len() { - bail!("certificate table lies outside the PE file"); - } let (cert_start, cert_end) = if l.cert_table_size == 0 { (pe.len(), pe.len()) } else { + let cert_end = l + .cert_table_off + .checked_add(l.cert_table_size) + .context("certificate table length overflows")?; + if l.cert_table_off > pe.len() || cert_end > pe.len() { + bail!("certificate table lies outside the PE file"); + } (l.cert_table_off, cert_end) }; let mut h = Sha256::new(); @@ -433,6 +433,18 @@ mod tests { assert!(pe_hash(&pe).is_err()); } + #[test] + fn ignores_a_stale_certificate_offset_when_the_table_is_empty() { + let mut pe = vec![0u8; 0x200]; + pe[..2].copy_from_slice(b"MZ"); + pe[0x3c..0x40].copy_from_slice(&(0x80u32).to_le_bytes()); + pe[0x80..0x84].copy_from_slice(b"PE\0\0"); + pe[0x98..0x9a].copy_from_slice(&[0x0b, 0x01]); + pe[0x118..0x11c].copy_from_slice(&(0xffff_fff0u32).to_le_bytes()); + pe[0x11c..0x120].copy_from_slice(&(0u32).to_le_bytes()); + assert!(pe_hash(&pe).is_ok()); + } + #[test] fn pe_hash_matches_osslsigncode() { let pe = include_bytes!("../tests/fixtures/hello.exe");