[LOW] Security hardening — miscellaneous low-severity findings

[austin-barrington]

Summary

Collection of low-severity / informational security findings that should be addressed as hardening work. Each is independently low risk but contributes to defense-in-depth.

Findings covered: #14, #15, #16, #17, #18, #19

#14 — Username-enumeration timing oracle

File: adapters/auth.rs:53-56

let stored = match self.metadata.get_user(username).await? {
    Some(s) => s,
    None => return Ok(None),  // early return, no argon2
};

Unknown users skip the CPU-heavy argon2 verify_password call. An attacker can distinguish valid from invalid usernames by measuring response time.

Fix: Always run argon2 against a dummy hash for non-existent users.

#15 — Credential cache keyed on weak hash

File: adapters/auth.rs:14-19

fn password_fingerprint(password: &str) -> u64 {
    let mut h = std::collections::hash_map::DefaultHasher::new();
    password.hash(&mut h);
    h.finish()
}

Cache key is a 64-bit DefaultHasher (SipHash-1-3) fingerprint of the password. Two passwords colliding on this hash share a cache entry. In practice, collisions on 64-bit hashes are extremely unlikely, so this is informational. Not a targeted bypass.

Fix: None required, but document the collision risk. Consider using the full hash string as the key if paranoia level warrants it.

#16 — Unchecked i64 timestamp multiply

File: domain/database.rs:193-200

pub fn to_nanos(&self, ts: i64) -> i64 {
    match self {
        Precision::Nanosecond => ts,
        Precision::Microsecond => ts * 1_000,
        Precision::Millisecond => ts * 1_000_000,
        Precision::Second => ts * 1_000_000_000,
    }
}

Uses plain * — silently wraps on overflow in release mode, panics in debug. A crafted timestamp near i64::MAX / 1_000_000_000 (~9.2e12) with Second precision overflows.

Fix: Use checked_mul and return Result or saturate.

#17 — WAL IPC length-prefix allocations

File: adapters/wal/wal_ipc.rs:167-175, 178-190

let fact_len = u32::from_le_bytes(len_buf) as usize;
let mut fact_ipc = vec![0u8; fact_len];  // up to ~4 GiB

decode_prepared_slot reads a u32 length from the WAL entry and allocates a buffer of that size without validating against remaining bytes. A corrupted WAL entry could declare ~4 GiB, triggering a massive allocation.

Scope: Local disk only (not network-reachable; peer sync uses JSON). Defense-in-depth.

Fix: Validate fact_len <= cursor.get_ref().len() - cursor.position() before allocating. Use bincode::Options::with_limit() for deserialization.

#18 — quote_backticks doesn't escape backslash

File: domain/chdb_naming.rs:54

pub fn quote_backticks(ident: &str) -> String {
    let escaped = ident.replace('`', '``');
    format!("`{}`", escaped)
}

Backslashes pass through unescaped. ClickHouse doesn't treat backslashes as escape characters inside backtick-quoted identifiers, so this is currently safe. All production callers pre-sanitize via sanitise_ident, which replaces non-[A-Za-z0-9_] bytes.

Fix: Escape \ → \\ for independence from caller pre-sanitization.

#19 — Hand-rolled base64 decoder + credentials in URLs

File: adapters/http/auth_middleware.rs:90-109

Hand-rolled base64_decode_bytes:

• No padding validation (= silently skipped)
• No output length validation
• O(n) scan per input byte against 64-byte table

Credentials accepted via ?u=&p= query parameters (lines 21-27, 57-63), which leak into server logs, browser history, and HTTP referrer headers. OWASP: "do not use URL query parameters for sensitive data."

Fix: Use the base64 or data-encoding crate. Deprecate query-param credentials in favor of Authorization header only.

Recommended Approach

These can be fixed in a single PR as a "security hardening" pass. None require breaking changes.