mirror of
https://github.com/microsoft/PowerToys.git
synced 2026-07-09 03:49:52 +02:00
## Summary of the Pull Request The File Locksmith IPC layer reads and writes raw UTF-16 (WCHAR) bytes to `last-run.log`, but all three stream opens were using the default text mode. On Windows, the CRT translates `0x0A` bytes to `0x0D 0x0A` on write and collapses `0x0D 0x0A` back to `0x0A` on read. Because each WCHAR is 2 bytes, any code unit whose little-endian byte pair contains `0x0A` in the low position (e.g. `U+010A`, `U+0A0D`) is silently corrupted. The fix opens all three streams in binary mode and adds an explicit open-failure guard. ```cpp // IPC.cpp — Writer::start() // Before m_stream = std::ofstream(path); // After m_stream = std::ofstream(path, std::ios::binary); // + is_open() guard returning E_FAIL on failure // NativeMethods.cpp — StartAsElevated() writer // Before std::ofstream stream(paths_file()); // After std::ofstream stream(paths_file(), std::ios::binary); // NativeMethods.cpp — ReadPathsFromFile() reader // Before std::ifstream stream(paths_file()); // After std::ifstream stream(paths_file(), std::ios::binary); ``` ## PR Checklist - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [ ] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx ## Detailed Description of the Pull Request / Additional comments Three targeted changes across two files: 1. **`FileLocksmithLib/IPC.cpp` — `Writer::start()`**: switched `std::ofstream` from text to binary mode and added an `is_open()` check that returns `E_FAIL` immediately when the file cannot be opened (previously the try/catch did not catch a silent open failure because `std::ofstream` does not throw by default). 2. **`FileLocksmithLibInterop/NativeMethods.cpp` — `StartAsElevated()`**: switched `std::ofstream` from text to binary mode. This is the elevated-restart writer path; without this fix, Unicode corruption persisted when File Locksmith relaunched as administrator. 3. **`FileLocksmithLibInterop/NativeMethods.cpp` — `ReadPathsFromFile()`**: switched `std::ifstream` from text to binary mode. This is the symmetric reader-side bug — even with both writers corrected, the CRT text-mode reader could collapse a `0x0D 0x0A` byte pair (a valid UTF-16 LE code unit, e.g. U+0A0D GURMUKHI EK ONKAR) into a single byte, desynchronising the 2-bytes-at-a-time read loop and corrupting all subsequent path data. No behaviour change for purely ASCII paths. Paths containing Unicode code points whose little-endian UTF-16 byte pair spans `0x0D 0x0A` were silently corrupted in all three code paths before this fix. ## Validation Steps Performed - Code review: no issues flagged. - Full diff reviewed: all three stream opens (`ofstream` writer in `IPC.cpp`, `ofstream` writer in `NativeMethods.cpp`, `ifstream` reader in `NativeMethods.cpp`) now use `std::ios::binary`, making write and read paths byte-exact and symmetric. - Mechanically correct: `std::ios::binary` suppresses Windows CRT `\n`↔`\r\n` translation; the delimiter `L'\n'` (LE bytes `0x0A 0x00`) is unambiguous in binary mode and is handled correctly by the existing read loop. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: MuyuanMS <116717757+MuyuanMS@users.noreply.github.com> Co-authored-by: Muyuan Li (from Dev Box) <muyuanli@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
209 lines
6.0 KiB
C++
209 lines
6.0 KiB
C++
#include "pch.h"
|
|
#include "NativeMethods.h"
|
|
#include "FileLocksmith.h"
|
|
#include "../FileLocksmithLib/Constants.h"
|
|
|
|
namespace winrt::PowerToys::FileLocksmithLib::Interop::implementation
|
|
{
|
|
|
|
#pragma region HelperMethods
|
|
std::wstring executable_path()
|
|
{
|
|
return pid_to_full_path(GetCurrentProcessId());
|
|
}
|
|
std::wstring paths_file()
|
|
{
|
|
std::wstring path{ PowerToys::Interop::Constants::AppDataPath() };
|
|
path += L"\\";
|
|
path += constants::nonlocalizable::PowerToyName;
|
|
path += L"\\";
|
|
path += constants::nonlocalizable::LastRunPath;
|
|
return path;
|
|
}
|
|
|
|
#pragma endregion
|
|
|
|
com_array<winrt::PowerToys::FileLocksmithLib::Interop::ProcessResult> NativeMethods::FindProcessesRecursive(array_view<hstring const> paths)
|
|
{
|
|
std::vector<std::wstring> paths_cpp{ paths.begin(), paths.end() };
|
|
|
|
auto result_cpp = find_processes_recursive(paths_cpp);
|
|
const auto result_size = static_cast<int>(result_cpp.size());
|
|
|
|
std::vector<ProcessResult> result;
|
|
|
|
if (result_size == 0)
|
|
{
|
|
return com_array<ProcessResult>();
|
|
}
|
|
|
|
for (int i = 0; i < result_size; i++)
|
|
{
|
|
result.push_back(ProcessResult
|
|
{
|
|
hstring { result_cpp[i].name },
|
|
result_cpp[i].pid,
|
|
hstring{ result_cpp[i].user },
|
|
winrt::com_array<hstring>
|
|
{
|
|
result_cpp[i].files.begin(), result_cpp[i].files.end()
|
|
}
|
|
});
|
|
}
|
|
|
|
return com_array<ProcessResult>{ result.begin(), result.end() };
|
|
}
|
|
|
|
hstring NativeMethods::PidToFullPath(uint32_t pid)
|
|
{
|
|
return hstring{ pid_to_full_path(pid) };
|
|
}
|
|
|
|
com_array<hstring> NativeMethods::ReadPathsFromFile()
|
|
{
|
|
std::ifstream stream(paths_file(), std::ios::binary);
|
|
|
|
std::vector<std::wstring> result_cpp;
|
|
std::wstring line;
|
|
|
|
bool finished = false;
|
|
|
|
while (!finished)
|
|
{
|
|
WCHAR ch{};
|
|
// We have to read data like this
|
|
if (!stream.read(reinterpret_cast<char*>(&ch), 2))
|
|
{
|
|
finished = true;
|
|
}
|
|
else if (ch == L'\n')
|
|
{
|
|
if (line.empty())
|
|
{
|
|
finished = true;
|
|
}
|
|
else
|
|
{
|
|
result_cpp.push_back(line);
|
|
line = {};
|
|
}
|
|
}
|
|
else
|
|
{
|
|
line += ch;
|
|
}
|
|
}
|
|
return com_array<hstring>{ result_cpp.begin(), result_cpp.end() };
|
|
}
|
|
|
|
bool NativeMethods::StartAsElevated(array_view<hstring const> paths)
|
|
{
|
|
std::ofstream stream(paths_file(), std::ios::binary);
|
|
const WCHAR newline = L'\n';
|
|
|
|
for (uint32_t i = 0; i < paths.size(); i++)
|
|
{
|
|
std::wstring path_cpp{ paths[i] };
|
|
stream.write(reinterpret_cast<const char*>(path_cpp.c_str()), path_cpp.size() * sizeof(WCHAR));
|
|
stream.write(reinterpret_cast<const char*>(&newline), sizeof(WCHAR));
|
|
}
|
|
|
|
stream.write(reinterpret_cast<const char*>(&newline), sizeof(WCHAR));
|
|
|
|
if (!stream)
|
|
{
|
|
return false;
|
|
}
|
|
|
|
stream.close();
|
|
|
|
auto exec_path = executable_path();
|
|
|
|
SHELLEXECUTEINFOW exec_info{};
|
|
exec_info.cbSize = sizeof(exec_info);
|
|
exec_info.fMask = SEE_MASK_NOCLOSEPROCESS;
|
|
exec_info.hwnd = NULL;
|
|
exec_info.lpVerb = L"runas";
|
|
exec_info.lpFile = exec_path.c_str();
|
|
exec_info.lpParameters = L"--elevated";
|
|
exec_info.lpDirectory = NULL;
|
|
exec_info.nShow = SW_SHOW;
|
|
exec_info.hInstApp = NULL;
|
|
|
|
if (ShellExecuteExW(&exec_info))
|
|
{
|
|
CloseHandle(exec_info.hProcess);
|
|
return true;
|
|
}
|
|
|
|
return false;
|
|
}
|
|
|
|
/* Adapted from "https://learn.microsoft.com/windows/win32/secauthz/enabling-and-disabling-privileges-in-c--" */
|
|
bool NativeMethods::SetDebugPrivilege()
|
|
{
|
|
HANDLE hToken;
|
|
TOKEN_PRIVILEGES tp{};
|
|
LUID luid;
|
|
|
|
if (OpenProcessToken(GetCurrentProcess(), TOKEN_ADJUST_PRIVILEGES, &hToken) != 0)
|
|
{
|
|
if (!LookupPrivilegeValue(
|
|
NULL, // lookup privilege on local system
|
|
SE_DEBUG_NAME, // privilege to lookup
|
|
&luid)) // receives LUID of privilege
|
|
{
|
|
CloseHandle(hToken);
|
|
return false;
|
|
}
|
|
tp.PrivilegeCount = 1;
|
|
tp.Privileges[0].Luid = luid;
|
|
tp.Privileges[0].Attributes = SE_PRIVILEGE_ENABLED;
|
|
|
|
if (!AdjustTokenPrivileges(
|
|
hToken,
|
|
FALSE,
|
|
&tp,
|
|
sizeof(TOKEN_PRIVILEGES),
|
|
NULL,
|
|
NULL))
|
|
{
|
|
CloseHandle(hToken);
|
|
return false;
|
|
}
|
|
|
|
if (GetLastError() == ERROR_NOT_ALL_ASSIGNED)
|
|
{
|
|
CloseHandle(hToken);
|
|
return false;
|
|
}
|
|
|
|
CloseHandle(hToken);
|
|
return true;
|
|
}
|
|
return false;
|
|
}
|
|
|
|
// adapted from common/utils/elevation.h. No need to bring all dependencies to this project, though.
|
|
// TODO: Make elevation.h lighter so that this function can be used without bringing dependencies like spdlog in.
|
|
bool NativeMethods::IsProcessElevated()
|
|
{
|
|
HANDLE token = nullptr;
|
|
bool elevated = false;
|
|
if (OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &token))
|
|
{
|
|
TOKEN_ELEVATION elevation{};
|
|
DWORD size;
|
|
if (GetTokenInformation(token, TokenElevation, &elevation, sizeof(elevation), &size))
|
|
{
|
|
elevated = (elevation.TokenIsElevated != 0);
|
|
}
|
|
}
|
|
if (token)
|
|
{
|
|
CloseHandle(token);
|
|
}
|
|
return elevated;
|
|
}
|
|
}
|