Maintainer assessment

Fixed in #151 and released in 0.1.2. Additional finding (Medium), found during triage: the host trusted a Paired or AutoPaired handshake result when no credential had been presented, contrary to the THP specification. A BLE imposter the user connects to could skip pairing and then receive a host-entered passphrase or transaction data, or return forged addresses. It is now rejected. Finding 4 is more significant than reported: trezor-thp does not gate create_session, so a direct caller could send a passphrase during an unauthenticated pairing phase. Finding 3 did not reproduce as described: a peer without the device's static private key selects no credential or fails the Noise tag check, so it cannot erase a stored credential. Also fixed: an omitted Ethereum data field was signed as one 0x00 calldata byte instead of empty calldata.

Original report

Summary

Four Low-severity findings in hw-core's host-side THP (Trezor Host Protocol) pairing/storage layer and per-chain signing-request builder. None breaks the device-side display/signing security guarantees — the secure-channel crypto, replay protection and command/response binding that matter most live in the external trezor-thp crate and in firmware, which are both out of this repo's scope. These are host-side hardening gaps worth fixing.

Finding 1 — Host THP static private key and pairing credentials persisted as plaintext JSON

Severity: Low crates/trezor-connect/src/thp/storage/snapshot.rs:14-19:

pub struct HostSnapshot {
    pub schema_version: u32,
    pub static_key: Option<Vec<u8>>,          // host THP static PRIVATE key, raw
    pub known_credentials: Vec<KnownCredential>,
}

crates/trezor-connect/src/thp/storage/file_storage.rs writes this struct verbatim as pretty-printed JSON (e.g. to ~/.hw-core/thp-host.json). Anyone who can read that file (local code execution, a backup, a cloud-synced home directory, another process running as the same user) can resume this host's paired identity against the real device without re-pairing. The directory/file write path itself is unusually hardened (owner-uid checks, rejected group/other write bits, extended-ACL scrubbing, O_NOFOLLOW, atomic renameat), and the struct's own doc comment explicitly tells integrators the snapshot is secret and should go in secure storage — so this is a documented, known property of the default FileStorage backend, not a hidden flaw. Signing requests still require on-device user confirmation, so stealing this file does not by itself move funds. The residual gap is specifically against other same-machine processes, cold backups, and synced home directories — exactly the case Trezor Suite avoids by using the OS keychain instead of a plain file. Fix: store static_key (and ideally known_credentials) via the OS keychain/credential manager where available, falling back to the current hardened file storage only when no keychain integration exists, and make that fallback explicit to the integrator.

Finding 2 — hex::decode silently left-pads odd-length hex instead of rejecting it

Severity: Low crates/hw-wallet/src/hex.rs:25-31:

let padded = if !stripped.len().is_multiple_of(2) {
    format!("0{stripped}")
} else {
    stripped.to_owned()
};
::hex::decode(&padded)...

An odd-length hex string (e.g. "123", or a truncated/typo'd caller value) is silently left-padded with a leading zero nibble instead of being rejected — "123" decodes to [0x01, 0x23] rather than returning an error. This feeds Ethereum calldata, Bitcoin script/witness/OP_RETURN data, and SLIP-24 signature/MAC fields throughout crates/hw-wallet/src/{eth,btc}/. The device still signs and displays the exact bytes that result — there is no divergence between what the device shows and what gets signed, so this cannot be used to make a user approve something different from what they see. The party actually misled is the SDK integrator, whose malformed/truncated input silently becomes different, valid-looking bytes instead of an error at the API boundary. Fix: reject odd-length hex input instead of padding it; require callers to supply well-formed, even-length hex.

Finding 3 — Device-controlled response permanently erases the stored pairing credential for the real device

Severity: Low crates/trezor-connect/src/thp/workflow/handshake.rs:88-93,106:

HandshakeCompletionState::RequiresPairing => {
    if let Some(selected) = selected_credential {
        self.config.forget_credential(&selected.credential);
    }
    ...
}
...
self.persist_host_state().await?;

Any BLE device that advertises as the target Trezor but completes the Noise handshake as an unrecognized device causes the host to permanently delete and persist the deletion of the stored pairing credential for the genuine, previously-paired device — before the user has done anything beyond connecting. This requires the user to actually connect to an attacker-controlled device advertising under the target's identity, and the device itself still requires explicit on-device user confirmation for any subsequent pairing/signing action, so the practical impact is a denial-of-service (forced re-pairing nuisance) rather than a path to unauthorized signing. Deleting a credential the connected device demonstrably does not recognize is also reasonable state-consistency behavior in isolation — the gap is that it happens unconditionally and persists immediately, with no distinction between "the real device reset its pairing" and "some other BLE peripheral answered." Fix: before erasing a stored credential, consider requiring additional confirmation (e.g. only erase after a user-confirmed re-pair succeeds) rather than erasing immediately on any RequiresPairing response from an unverified peer identity.

Finding 4 — create_session is missing the phase guard every other workflow entry point has

Severity: Low crates/trezor-connect/src/thp/workflow/session.rs:15-29:

pub async fn create_session(&mut self, passphrase: Option<String>, ...) -> Result<()> {
    self.backend.create_new_session(CreateSessionRequest { ... }).await?;
    Ok(())
}

Every other security-relevant workflow entry point calls require_phase(Phase::Paired) before proceeding (workflow/signing.rs:17,25,41, workflow/session.rs:32,37,42) — create_session does not, so it is reachable while the workflow state machine still considers itself to be in Handshake/Pairing. The underlying trezor-thp channel and the device itself independently reject session commands sent outside an established, paired, encrypted channel, so the realistic effect is an inconsistent error path rather than a security bypass — but it is the one entry point in this file missing a check every sibling function has, worth closing for consistency. Fix: add the same require_phase(Phase::Paired) guard to create_session that every other entry point in this module already has. Best regards, Gleb Lizunov