fix: a library comes from the system directory, and the catalogue from one place

[donislawdev]

Findings S7 and S9 of the outside security review. The first was wrong about the fix, right about the risk, and the guard written to hold that risk found a second call the review never looked at.

S7: the proposed fix does not exist, and the real one was elsewhere

The review asked for syscall.NewLazySystemDLL instead of NewLazyDLL in the free space lookup, saying it is in the standard library.

It is not. Measured: the compiler answers undefined: syscall.NewLazySystemDLL, and syscall has no LoadLibraryEx and no LOAD_LIBRARY_SEARCH_ constants either. The System form lives in golang.org/x/sys/windows, which untouchable rule 11 keeps out of the command line binary - the comment beside that call has said so since 2026-08-25.

And that call is safe as written. kernel32.dll is a KnownDLL: measured in the registry, 37 entries, *kernel32 = kernel32.dll. A KnownDLL is already mapped, so the loader hands back what is there and never consults a search order.

So the finding is about the next call, not that one - and the guard written to hold that found it:

🔴 internal/gui loaded uxtheme.dll by name, and uxtheme.dll is not a KnownDLL - same registry key, not among the 37. The Windows search order puts the directory the program was started from before the system one, so a file of that name left beside a downloaded tfg-gui.exe would have been loaded into the window and run.

It now comes from an absolute path under the system directory, asked for through kernel32 - the one library that cannot be diverted. Checked on a real system rather than by reading: the guard that asks Windows for dark menus calls it twice and still gets them.

⚠️ That change moved the library's name out of the syscall call and into a helper of ours, where the no-telemetry scan would have walked straight past it. One shared list of loading calls now covers both shapes, and our helper is on it. A guard that gets safer code and stops looking is worse than the code it was guarding.

S9: accepted as a fact, refused as a remedy

The localiser is a package variable with no lock: Load writes it, and say, sayf and sayN read it on every string the window draws. Safe because it happens once, before any screen exists - a constraint rather than a property of the code.

The review offered an atomic.Pointer or writing the constraint down. An atomic pointer here would be an eighth defence nothing in this build could redden, and seven such pieces have been removed from this project for exactly that reason.

So the constraint is mechanical instead: a guard fails when a second caller appears. Whoever adds the language switch - which LoadBuiltIn's own comment names as its own piece of work - starts from the sentence on the variable rather than discovering the race. The race detector would not necessarily find it either, because fyne.Do runs on the calling goroutine under the test driver.

Guards

Two new, both with mutations, plus one existing mutation repaired and the no-telemetry registry extended. All caught.

🤖 Generated with Claude Code