mirror of
https://github.com/microsoft/PowerToys.git
synced 2026-08-29 01:59:34 +02:00
Fix _snwprintf_s size argument in BugReportTool EventViewer (#48398)
## Summary Caught while reading through `BugReportTool` for an unrelated review: the two `_snwprintf_s` calls in `EventViewer.cpp` pass `sizeof(buff)` as the buffer-size argument, but `buff` is a `wchar_t[1000]`. `_snwprintf_s` measures its size and count arguments in **wide characters**, not bytes, so the current code advertises a 2000-wchar destination for a buffer that only holds 1000. `cpp wchar_t buff[1000]; // 2000 bytes, 1000 wchars memset(buff, 0, sizeof(buff)); _snwprintf_s(buff, sizeof(buff), fmt, ...); // <-- 2000 passed as wchar count ` If the formatted output ever exceeds 1000 wchars, the Secure CRT bounds check fires (in debug) and - depending on which `_snwprintf_s` overload the compiler selects against the safe template - it can write past the end of the stack buffer in release. Neither format string here is likely to produce 1000+ characters in practice (one substitutes a process name, the other a channel name + integer), so this is more of a latent footgun than a known crash, but the bounds are simply wrong. ## Fix Use `_countof(buff)` for the size argument (which is what `_snwprintf_s` actually wants - element count, not byte count) and pass `_TRUNCATE` for the count so output is safely capped at 999 wchars plus the null terminator: `cpp _snwprintf_s(buff, _countof(buff), _TRUNCATE, fmt, ...); ` Applied to both `GetQuery` and `GetQueryByChannel`. ## Scope Searched the rest of the repo for the same pattern (`_snwprintf_s(buf, sizeof(...))` / `_snprintf_s(buf, sizeof(...))`) - these two call sites are the only occurrences in the codebase. ## Validation - `BugReportTool.sln` rebuilds clean locally (Release|x64) and produces `PowerToys.BugReportTool.exe`. - No behavior change on the happy path - both formats are well under 1000 wchars in normal use. ## Risk Low. Two-line change in a single utility that builds event-log queries for bug reports. Truncation on overflow is strictly safer than the prior behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -45,7 +45,7 @@ namespace
|
||||
{
|
||||
wchar_t buff[1000];
|
||||
memset(buff, 0, sizeof(buff));
|
||||
_snwprintf_s(buff, sizeof(buff), QUERY_BY_PROCESS.c_str(), PERIOD, processName.c_str());
|
||||
_snwprintf_s(buff, _countof(buff), _TRUNCATE, QUERY_BY_PROCESS.c_str(), PERIOD, processName.c_str());
|
||||
return buff;
|
||||
}
|
||||
|
||||
@@ -53,7 +53,7 @@ namespace
|
||||
{
|
||||
wchar_t buff[1000];
|
||||
memset(buff, 0, sizeof(buff));
|
||||
_snwprintf_s(buff, sizeof(buff), QUERY_BY_CHANNEL.c_str(), channelName.c_str(), PERIOD);
|
||||
_snwprintf_s(buff, _countof(buff), _TRUNCATE, QUERY_BY_CHANNEL.c_str(), channelName.c_str(), PERIOD);
|
||||
return buff;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user