globalPin is now initialised to L"" so any bypass path that skips the
dialog produces an empty string that validation will reject. The
"0000" default is instead passed to ShowInputBox as defaultValue and
pre-filled into the edit control via SetDlgItemText on WM_INITDIALOG,
preserving the UX the author intended. ShowInputBox now also returns
L"" when the user cancels instead of leaking the previous static buffer
contents.
Fixes#15
Issue #12: IDC_BTN_SHOW_SELECTED executed a -delete command silently
with no confirmation. Renamed the ID to IDC_BTN_DELETE_PASSKEY in
Resource.h and the case label to reflect the actual behaviour. Added
a MB_YESNO confirmation dialog showing the credential ID before any
delete is executed. MB_DEFBUTTON2 makes No the default so pressing
Enter cannot accidentally destroy a passkey.
Issue #13: Removed all dead code accumulated during development:
- 20+ commented-out MessageBox debug calls and their orphaned
'// Debug:' label comments throughout the file
- 6 for-loops whose entire body was a commented-out MessageBox
(loops in GetDomains, GetResidentKeysWithDomains, PasskeysDialogProc
refresh handler, and the WM_COMMAND passkeys handler)
- Empty else{} block in FingerprintDialogProc that held only a debug
comment
- #include <thread> which was never used
- 5 redundant forward declarations (RunCommandAndGetOutput,
PopulateComboBox, PopulateListView, ResizeControls, RefreshData)
whose definitions all appear before any call site; WindowProc
forward declaration kept as it is genuinely needed
Fixes#12Fixes#13
EscapeCommandLineArgument prefixed shell metacharacters with ^ which
is cmd.exe syntax. RunCommandAndGetOutput calls CreateProcess directly,
so ^ characters were passed verbatim to fido2-manage.exe, corrupting
the PIN for any input containing those characters.
Replaced the function with QuoteArg which follows the Windows C-runtime
argument parsing rules: wrap in double quotes, double backslashes that
precede a double quote or the closing quote, and escape embedded double
quotes with a backslash.
Also fixed two related problems:
- The PIN length check was running against the already-escaped value.
A 2-char PIN escaped to a 4-char quoted string would silently pass.
Moved the check before QuoteArg so it validates the raw input.
- deviceNumber and domain were never escaped in any command string.
Applied QuoteArg inline at all seven CreateProcess command-building
sites. ShellExecute paths are left unchanged as they go through
cmd.exe and deviceNumber is always regex-validated digits.
Fixes#2
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