From 2384913853c4412cc6027d5c81f35f4cc6c4cd05 Mon Sep 17 00:00:00 2001 From: matthew-pilot Date: Thu, 28 May 2026 06:32:29 +0000 Subject: [PATCH] fix: close TOCTOU in LoadLocalSigner and LoadEVMSigner (PILOT-86) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace os.Stat + os.ReadFile with os.Open + fd.Stat + io.ReadAll to eliminate the time-of-check/time-of-use window where an attacker can chmod the identity file between the permission check and the read. pkg/wallet/signer.go LoadLocalSigner: os.Stat→fd.Stat on opened fd pkg/evm/key.go LoadEVMSigner: same pattern All three operations (stat, validate, read) now target the same inode handle — the fd guarantees identity of the checked file. Tests: 5/5 packages pass (go test ./...) Vet: clean --- pkg/evm/key.go | 22 +++++++++++++++++----- pkg/wallet/signer.go | 25 ++++++++++++++++++++----- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/pkg/evm/key.go b/pkg/evm/key.go index da5398c..daf5b7d 100644 --- a/pkg/evm/key.go +++ b/pkg/evm/key.go @@ -14,6 +14,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "os" "path/filepath" "strings" @@ -281,12 +282,23 @@ func LoadOrCreateEVMSigner(path string) (*EVMSigner, error) { // secp256k1 seed lets any reader move funds, so the OpenSSH-style // permissions check is non-optional). func LoadEVMSigner(path string) (*EVMSigner, error) { - if info, err := os.Stat(path); err == nil { - if perm := info.Mode().Perm(); perm&0o077 != 0 { - return nil, fmt.Errorf("evm: identity %s: permissions %#o expose the seed to other users — chmod 0600 to fix", path, perm) - } + // Open-and-fstat-then-read to close the TOCTOU window between + // os.Stat and os.ReadFile (same rationale as LoadLocalSigner). + fd, err := os.Open(path) + if err != nil { + return nil, err + } + defer fd.Close() + + info, err := fd.Stat() + if err != nil { + return nil, fmt.Errorf("evm: identity %s: stat: %w", path, err) } - raw, err := os.ReadFile(path) + if perm := info.Mode().Perm(); perm&0o077 != 0 { + return nil, fmt.Errorf("evm: identity %s: permissions %#o expose the seed to other users — chmod 0600 to fix", path, perm) + } + + raw, err := io.ReadAll(fd) if err != nil { return nil, err } diff --git a/pkg/wallet/signer.go b/pkg/wallet/signer.go index 13ab2b1..b7a15e2 100644 --- a/pkg/wallet/signer.go +++ b/pkg/wallet/signer.go @@ -7,6 +7,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "os" "path/filepath" "time" @@ -94,12 +95,26 @@ func LoadOrCreateLocalSigner(path string) (*LocalSigner, error) { // identity file would silently let any local user read the seed and // impersonate the wallet. func LoadLocalSigner(path string) (*LocalSigner, error) { - if info, err := os.Stat(path); err == nil { - if perm := info.Mode().Perm(); perm&0o077 != 0 { - return nil, fmt.Errorf("identity %s: permissions %#o expose the seed to other users — chmod 0600 to fix", path, perm) - } + // Open-and-fstat-then-read to close the TOCTOU window between + // os.Stat and os.ReadFile. An attacker who races a chmod 0644 + // between the check and the read sees the mode validation fail + // because the fstat targets the already-opened fd, not the + // path. + fd, err := os.Open(path) + if err != nil { + return nil, err + } + defer fd.Close() + + info, err := fd.Stat() + if err != nil { + return nil, fmt.Errorf("identity %s: stat: %w", path, err) } - data, err := os.ReadFile(path) + if perm := info.Mode().Perm(); perm&0o077 != 0 { + return nil, fmt.Errorf("identity %s: permissions %#o expose the seed to other users — chmod 0600 to fix", path, perm) + } + + data, err := io.ReadAll(fd) if err != nil { return nil, err }