[Awake] WM_COMMAND message processing flaw #41918
Description
Microsoft PowerToys version
0.94
Installation method
Dev build in Visual Studio
Area(s) with issue?
Awake
Steps to reproduce
Awake sets up a system tray menu and processes custom messages to perform its functions, such as switching between timed modes, setting the display mode to always on, and exiting. The processing of these messages has a flaw.
The bug
The custom tray commands that Awake responds to are declared in Core\Models\TrayCommands.cs:
internal enum TrayCommands : uint
{
TC_DISPLAY_SETTING = Native.Constants.WM_USER + 0x2,
TC_MODE_PASSIVE = Native.Constants.WM_USER + 0x3,
TC_MODE_INDEFINITE = Native.Constants.WM_USER + 0x4,
TC_MODE_EXPIRABLE = Native.Constants.WM_USER + 0x5,
TC_EXIT = Native.Constants.WM_USER + 0x64,
TC_TIME = Native.Constants.WM_USER + 0x65,
}(The WM_USER constant corresponds to the start of user-definable custom messages and is set to 0x400, meaning TC_DISPLAY_SETTING evaluates to 0x402, TC_MODE_PASSIVE is 0x403 and so on.)
TC_TIME is used as a placeholder to signal the start of the custom times defined in the settings file. By default, these are 30 minutes (TC_TIME + 0), 1 hour (TC_TIME + 1) and 2 hours (TC_TIME + 2).
The issue is when the custom commands are checked in the TrayHelper's WndProc routine:
case Native.Constants.WM_COMMAND:
int trayCommandsSize = Enum.GetNames<TrayCommands>().Length;
long targetCommandIndex = wParam.ToInt64() & 0xFFFF;
switch (targetCommandIndex)
{
// Code for responding to TC_EXIT, TC_DISPLAY_SETTING, TC_MODE_INDEFINITE, TC_MODE_PASSIVE removed...
default:
{
if (targetCommandIndex >= trayCommandsSize)
{
AwakeSettings settings = Manager.ModuleSettings!.GetSettings<AwakeSettings>(Constants.AppName);
if (settings.Properties.CustomTrayTimes.Count == 0)
{
settings.Properties.CustomTrayTimes.AddRange(Manager.GetDefaultTrayOptions());
}
int index = (int)targetCommandIndex - (int)TrayCommands.TC_TIME;
uint targetTime = settings.Properties.CustomTrayTimes.ElementAt(index).Value;
Manager.SetTimedKeepAwake(targetTime, keepDisplayOn: settings.Properties.KeepDisplayOn);
}
break;
}The core problem lies with a mix-up between the cardinality of the commands enum and the values it contains.
int trayCommandsSize = Enum.GetNames<TrayCommands>().Length; gets the number of items in the TrayCommands enum, which is 6.
In the default block we have if (targetCommandIndex >= trayCommandsSize) which tests the value of the command against 6. However, custom commands (and therefore the value in targetCommandIndex) start at 0x402 (TC_DISPLAY_SETTINGS is WM_USER + 0x2), so the condition will always succeed. The name targetCommandIndex suggests that it was intended to hold the index of the enum entry instead of the value.
There is no check that the calculated index is valid, so anArgumentOutOfRangeException may be thrown when the settings.Properties.CustomTrayTimes.ElementAt(index).Value code is run.
Fortunately, in the current version of Awake, each of the valid commands added to the tray menu is checked for before the default block, so the error cannot occur (TC_MODE_EXPIRABLE is for the command-line mode only and isn't added to the system tray menu). By a process of elimination, the only messages left must correspond to the custom time entries. However, this is fragile and the error could be triggered easily if a new command were added to the menu but not explicitly checked for in WndProc. The lack of error handling means that it may be challenging to spot, as the application would immediately exit.
Proposed fix
Rewrite the handler for WM_COMMAND to be:
case Native.Constants.WM_COMMAND:
long cmdId = wParam.ToInt64() & 0xFFFF;
// Remove trayCommandsSize, it is unneeded.
switch (cmdId)
{
// Cases for TC_EXIT, TC_DISPLAY_SETTING, TC_MODE_INDEFINITE and TC_MODE_PASSIVE are unchanged
// ...
default:
if (cmdId >= (uint)TrayCommands.TC_TIMES)
{
// The user selected a keep awake duration from the menu.
var userSettings = Manager.ModuleSettings.GetSettings<AwakeSettings>(Constants.AppName);
if (userSettings.Properties.CustomTrayTimes.Count == 0)
{
userSettings.Properties.CustomTrayTimes.AddRange(_defaultCustomTimes);
}
// The custom duration commands begin at TC_TIMES.
int index = (int)cmdId - (int)TrayCommands.TC_TIMES;
if (index < 0 || index >= userSettings.Properties.CustomTrayTimes.Count)
{
Logger.LogError($"Invalid index into CustomTrayTimes array. Index: {index}");
}
else
{
uint targetTime = userSettings.Properties.CustomTrayTimes.ElementAt(index).Value;
Manager.SetTimedKeepAwake(targetTime, keepDisplayOn: userSettings.Properties.KeepDisplayOn);
}
}
else
{
// A command was received for which we do not have a handler.
Logger.LogError($"Unknown Windows Message received by TrayHelper WndProc. Message ID: {cmdId:X}. Ignoring.");
}
break;
}It would also be useful to add comments to the TrayCommands enum to describe that the last entry denotes the start of a range, as this isn't currently obvious. It may also be good to rename it to something more descriptive:
/// <summary>
/// Custom message IDs for Awake's system tray actions.
/// </summary>
internal enum TrayCommands : uint
{
TC_DISPLAY_SETTING = Native.Constants.WM_USER + 0x2,
TC_MODE_PASSIVE = Native.Constants.WM_USER + 0x3,
TC_MODE_INDEFINITE = Native.Constants.WM_USER + 0x4,
TC_MODE_EXPIRABLE = Native.Constants.WM_USER + 0x5,
TC_EXIT = Native.Constants.WM_USER + 0x64,
// Start of a contiguous range of custom time items.
// Do not place any enum values after this one.
TC_CUSTOM_TIMES_START = Native.Constants.WM_USER + 0x100,
}✔️ Expected Behavior
N/A
❌ Actual Behavior
N/A
Additional Information
No response
Other Software
No response