wcscpy_s into a 256-char stack buffer crashes via the invalid
parameter handler if a credential ID, fingerprint data, or device
info value exceeds 255 characters. FIDO2 credential IDs are binary
blobs encoded as hex or base64 and can easily exceed this limit.
Dropped the intermediate nameBuffer/valueBuffer in PopulateListView
and indexBuffer/dataBuffer in FingerprintDialogProc. Assigned
const_cast<LPWSTR>(str.c_str()) directly to lvItem.pszText instead,
which is safe because SendMessage(LVM_INSERTITEM/LVM_SETITEM) is
synchronous and copies the text before returning. This is the same
pattern already used correctly in PopulatePasskeysList.
Fixes#9
The forward declaration at line 568 had one parameter
(const std::wstring&) while the definition at line 694 takes two
(HWND, const std::wstring&). The mismatch is a latent ODR violation:
the compiler currently resolves correctly because the declaration
appears after the definition, but any reorganisation that moves the
declaration above the definition would cause a compile error or a
silent wrong-function call. Updated the declaration to match the
definition and the single call site exactly.
Fixes#8
The static buffer in ShowInputBox was only initialised once at program
start, never between calls. Pressing Cancel left the previous PIN in
the buffer, which was returned and silently used as globalPin on the
next device selection. Two changes made:
1. ShowInputBox: zero buffer[0] at the top of every call so a cancelled
dialog can never return stale data; capture DialogBoxParam return
value and return an empty string on anything other than IDOK.
2. PopulateListView caller: check globalPin.empty() before escaping
and exit silently — previously a cancel fell through to the length
check and showed a misleading 'PIN too short' error instead of just
resetting the UI.
Fixes#7
In both RunCommandAndGetOutput and RunCommandAndGetOutputWithTimeout,
hPipeWrite was only closed inside the CreateProcess success branch.
On failure the write handle leaked, consuming a kernel handle slot on
every failed invocation. Captured the CreateProcess return value into
a bool and moved CloseHandle(hPipeWrite) to run unconditionally after
the call, before the success check, which is the standard Windows
pipe pattern.
Fixes#6
RunCommandAndGetOutputWithTimeout was implemented but never called;
all 7 call sites used the no-timeout RunCommandAndGetOutput, leaving
the UI able to freeze permanently if fido2-manage.exe hangs or a
device is removed mid-operation. Added DEVICE_TIMEOUT_MS constant
(15 s) and routed every call site through the timeout function so
a stalled subprocess is killed and the UI recovers cleanly.
Fixes#5
The global hwnd was shadowed by a local variable declaration in
_tWinMain, leaving the global permanently NULL. RefreshData() passed
this NULL handle to DisableAllButtons(), dereferencing it on every
Refresh click. Removing the local type specifier assigns the created
window to the global so all callers outside _tWinMain see a valid handle.
Fixes#4
Token2
Tamir Suliman
Token2