From 1dbed02c20eadd7bff26bd6f86acca9c16d3114c Mon Sep 17 00:00:00 2001 From: Tamir Suliman Date: Sat, 6 Jun 2026 13:52:46 +0200 Subject: [PATCH] Fix #2 - replace CMD shell escaping with correct CreateProcess quoting 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 --- FIDO2.1 Manager.cpp | 77 +++++++++++++++++++-------------------------- 1 file changed, 33 insertions(+), 44 deletions(-) diff --git a/FIDO2.1 Manager.cpp b/FIDO2.1 Manager.cpp index 2d552e1..8c3745a 100644 --- a/FIDO2.1 Manager.cpp +++ b/FIDO2.1 Manager.cpp @@ -25,42 +25,32 @@ struct PasskeyInfo { std::wstring domain; }; -std::wstring EscapeCommandLineArgument(const std::wstring& input) { - std::wstring escaped = L"\""; // Start with a quote to properly handle spaces +// Quotes one argument for a CreateProcess command string using the +// Windows C-runtime parsing rules: wrap in double quotes, double any +// backslashes that immediately precede a double quote or the closing +// quote, and escape embedded double quotes with a backslash. +// Do NOT use this for cmd.exe shell strings (ShellExecute paths). +std::wstring QuoteArg(const std::wstring& arg) { + std::wstring result = L"\""; + int backslashes = 0; - for (wchar_t ch : input) { - switch (ch) { - case L'"': escaped += L"\\\""; break; // Escape double quotes - case L'^': - case L'&': - case L'|': - case L'<': - case L'>': - case L'%': - case L'!': - case L'(': - case L')': - case L'=': - case L';': - case L'`': - case L',': - case L'[': - case L']': - case L'{': - case L'}': - case L'*': - case L'?': - case L'\\': // Escape special characters for CMD - escaped += L'^'; - escaped += ch; - break; - default: - escaped += ch; + for (wchar_t c : arg) { + if (c == L'\\') { + backslashes++; + } else if (c == L'"') { + result.append(backslashes * 2, L'\\'); + result += L"\\\""; + backslashes = 0; + } else { + result.append(backslashes, L'\\'); + result += c; + backslashes = 0; } } - escaped += L"\""; // End with a quote - return escaped; + result.append(backslashes * 2, L'\\'); + result += L'"'; + return result; } @@ -137,7 +127,7 @@ std::vector ParseResidentKeys(const std::vector& outp std::vector GetDomains(const std::wstring& deviceNumber, const std::wstring& globalPin) { // Command for the first run - std::wstring command = L".\\fido2-manage.exe -residentkeys -device " + deviceNumber + L" -pin " + globalPin; + std::wstring command = L".\\fido2-manage.exe -residentkeys -device " + QuoteArg(deviceNumber) + L" -pin " + globalPin; // Debug: Show the command being executed //MessageBox(NULL, command.c_str(), L"Debug: Command Being Executed", MB_OK); @@ -168,8 +158,8 @@ std::vector GetResidentKeysWithDomains(const std::wstring& deviceNu for (const auto& domain : domains) { // Command for the second run - std::wstring command = L".\\fido2-manage.exe -residentkeys -device " + deviceNumber + - L" -domain " + domain + L" -pin " + globalPin; + std::wstring command = L".\\fido2-manage.exe -residentkeys -device " + QuoteArg(deviceNumber) + + L" -domain " + QuoteArg(domain) + L" -pin " + globalPin; std::vector output = RunCommandAndGetOutput(command); // Debug: Check the raw output @@ -302,7 +292,7 @@ INT_PTR CALLBACK PasskeysDialogProc(HWND hDlg, UINT message, WPARAM wParam, LPAR // Step 1: First Run - Fetch domains - std::wstring firstCommand = L".\\fido2-manage.exe -residentkeys -device " + deviceNumber + L" -pin " + globalPin; + std::wstring firstCommand = L".\\fido2-manage.exe -residentkeys -device " + QuoteArg(deviceNumber) + L" -pin " + globalPin; std::vector firstOutput = RunCommandAndGetOutput(firstCommand); // Parse the output to get domains (users) @@ -324,8 +314,8 @@ INT_PTR CALLBACK PasskeysDialogProc(HWND hDlg, UINT message, WPARAM wParam, LPAR // Step 2: Second Run - Fetch resident keys for each domain std::vector refreshedPasskeys; for (const auto& domain : domains) { - std::wstring secondCommand = L".\\fido2-manage.exe -residentkeys -device " + deviceNumber + - L" -domain " + domain + L" -pin " + globalPin; + std::wstring secondCommand = L".\\fido2-manage.exe -residentkeys -device " + QuoteArg(deviceNumber) + + L" -domain " + QuoteArg(domain) + L" -pin " + globalPin; std::vector secondOutput = RunCommandAndGetOutput(secondCommand); // Parse the second run output @@ -458,7 +448,7 @@ INT_PTR CALLBACK FingerprintDialogProc(HWND hDlg, UINT message, WPARAM wParam, L SendMessage(hListView, LVM_INSERTCOLUMN, 1, (LPARAM)&lvColumn); // Populate the ListView with fingerprints - std::wstring command = L".\\fido2-manage.exe -fingerprintlist -device " + deviceNumber + L" -pin " + globalPin; + std::wstring command = L".\\fido2-manage.exe -fingerprintlist -device " + QuoteArg(deviceNumber) + L" -pin " + globalPin; std::vector output = RunCommandAndGetOutput(command); for (const auto& line : output) { @@ -698,7 +688,6 @@ void PopulateListView(HWND hwnd, const std::wstring& deviceNumber) { DisableAllButtons(hwnd); // Show the input box and store the result in globalPIN globalPin = ShowInputBox(NULL, L"Enter PIN", L"Please enter your PIN:"); - globalPin = EscapeCommandLineArgument(globalPin); const size_t MIN_PIN_LENGTH = 4; @@ -706,15 +695,15 @@ void PopulateListView(HWND hwnd, const std::wstring& deviceNumber) { // Display an error message MessageBoxW(NULL, L"Error: PIN must be at least 4 characters long.", L"Invalid PIN", MB_OK | MB_ICONERROR); - // Terminate the application gracefully RefreshData(); return; - } + globalPin = QuoteArg(globalPin); + // Construct the command - std::wstring command = L".\\fido2-manage.exe -storage -device " + deviceNumber + L" -pin " + globalPin; + std::wstring command = L".\\fido2-manage.exe -storage -device " + QuoteArg(deviceNumber) + L" -pin " + globalPin; // Execute the command std::vector output = RunCommandAndGetOutput(command); @@ -741,7 +730,7 @@ void PopulateListView(HWND hwnd, const std::wstring& deviceNumber) { } // Construct the second command - std::wstring command2 = L".\\fido2-manage.exe -info -device " + deviceNumber; + std::wstring command2 = L".\\fido2-manage.exe -info -device " + QuoteArg(deviceNumber); // Execute the second command std::vector additionalOutput = RunCommandAndGetOutput(command2);