From 6776373e8ef8db23eae68f779b6b6bac4b1888bf Mon Sep 17 00:00:00 2001 From: fettpl <38704082+fettpl@users.noreply.github.com> Date: Sun, 15 Feb 2026 00:01:23 +0100 Subject: [PATCH] fix: constant_time_eq no longer leaks secret length via early return Remove the early return on length mismatch that leaked length information via timing. Now iterates over max(a.len(), b.len()), padding the shorter input with zeros, and checks both byte-level differences and length equality at the end. Closes #57 Co-Authored-By: Claude Opus 4.6 --- src/security/pairing.rs | 26 +++++++++++++++++++------- src/security/secrets.rs | 2 +- src/tools/browser.rs | 1 + 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/security/pairing.rs b/src/security/pairing.rs index 5f556035b..f9a9a05a3 100644 --- a/src/security/pairing.rs +++ b/src/security/pairing.rs @@ -174,15 +174,27 @@ fn generate_token() -> String { format!("zc_{}", uuid::Uuid::new_v4().as_simple()) } -/// Constant-time string comparison to prevent timing attacks on pairing code. +/// Constant-time string comparison to prevent timing attacks. +/// +/// Does not short-circuit on length mismatch — always iterates over the +/// longer input to avoid leaking length information via timing. pub fn constant_time_eq(a: &str, b: &str) -> bool { - if a.len() != b.len() { - return false; + let a = a.as_bytes(); + let b = b.as_bytes(); + + // Track length mismatch as a usize (non-zero = different lengths) + let len_diff = a.len() ^ b.len(); + + // XOR each byte, padding the shorter input with zeros. + // Iterates over max(a.len(), b.len()) to avoid timing differences. + let max_len = a.len().max(b.len()); + let mut byte_diff = 0u8; + for i in 0..max_len { + let x = if i < a.len() { a[i] } else { 0 }; + let y = if i < b.len() { b[i] } else { 0 }; + byte_diff |= x ^ y; } - a.bytes() - .zip(b.bytes()) - .fold(0u8, |acc, (x, y)| acc | (x ^ y)) - == 0 + len_diff == 0 && byte_diff == 0 } /// Check if a host string represents a non-localhost bind address. diff --git a/src/security/secrets.rs b/src/security/secrets.rs index bafad3856..394084303 100644 --- a/src/security/secrets.rs +++ b/src/security/secrets.rs @@ -241,7 +241,7 @@ fn hex_encode(data: &[u8]) -> String { /// Hex-decode a hex string to bytes. fn hex_decode(hex: &str) -> Result> { - if hex.len() % 2 != 0 { + if !hex.len().is_multiple_of(2) { anyhow::bail!("Hex string has odd length"); } (0..hex.len()) diff --git a/src/tools/browser.rs b/src/tools/browser.rs index 5ee95053d..25be13c2b 100644 --- a/src/tools/browser.rs +++ b/src/tools/browser.rs @@ -366,6 +366,7 @@ impl BrowserTool { } #[async_trait] +#[allow(clippy::too_many_lines)] impl Tool for BrowserTool { fn name(&self) -> &str { "browser"