diff --git a/src/WindowController.App/HotkeyManager.cs b/src/WindowController.App/HotkeyManager.cs index 0a60bcc..c653aa5 100644 --- a/src/WindowController.App/HotkeyManager.cs +++ b/src/WindowController.App/HotkeyManager.cs @@ -157,7 +157,10 @@ public HotkeyRegistrationResult TestHotkey(HotkeyBinding binding) // Try to register with a temporary ID that will not collide with real hotkey IDs. // Use unique IDs to avoid cross-thread interference. + // Guard against wraparound into the profile hotkey ID range. var testId = unchecked(_nextTestHotkeyId--); + if (_nextTestHotkeyId <= HOTKEY_ID_PROFILE_BASE + 100_000) + _nextTestHotkeyId = int.MaxValue; var result = NativeMethods.RegisterHotKey(hwnd, testId, modifiers | NativeMethods.MOD_NOREPEAT, (uint)vkCode); if (result) { diff --git a/src/WindowController.App/ProfileApplier.cs b/src/WindowController.App/ProfileApplier.cs index 1349a77..97943d0 100644 --- a/src/WindowController.App/ProfileApplier.cs +++ b/src/WindowController.App/ProfileApplier.cs @@ -150,6 +150,31 @@ private async Task ApplyProfileAsync(Profile profile, bool launchMi return new ApplyResult(applied, profile.Windows.Count, failures, warnings); } + // Extensions blocked from being launched via UseShellExecute to prevent + // arbitrary script execution through malicious profile data. + private static readonly HashSet BlockedExtensions = new(StringComparer.OrdinalIgnoreCase) + { + ".bat", ".cmd", ".ps1", ".psm1", ".psd1", + ".vbs", ".vbe", ".js", ".jse", ".wsf", ".wsh", + ".msi", ".msp", ".scr", ".hta", ".inf", ".reg", + ".com", ".pif" + }; + + private const int LaunchTimeoutMs = 12_000; + private const int LaunchPollIntervalMs = 300; + + /// + /// Returns true if the path points to a genuine executable (.exe) and not + /// a script or installer that could be used for arbitrary code execution. + /// + private static bool IsAllowedLaunchPath(string filePath) + { + var ext = Path.GetExtension(filePath); + if (BlockedExtensions.Contains(ext)) + return false; + return true; + } + private async Task LaunchAndWaitAsync(WindowEntry entry, List existingCandidates) { var exe = entry.Match.Exe; @@ -164,6 +189,13 @@ private async Task LaunchAndWaitAsync(WindowEntry entry, List LaunchAndWaitAsync(WindowEntry entry, List _lastMinMaxByHwnd = new(); private Dictionary _lastForegroundByProfile = new(); private Dictionary _lastForegroundTickByProfile = new(); - private bool _isPropagating; + private volatile bool _isPropagating; private long _lastRebuildTick; private bool _disposed; + // Protects _syncGroups, _profileNames, _lastMinMaxByHwnd, + // _lastForegroundByProfile, _lastForegroundTickByProfile against + // concurrent access from WinEvent callbacks and ScheduleRebuild (Task.Run). + private readonly object _syncLock = new(); + // Debounce rebuild: avoid multiple rapid rebuilds private CancellationTokenSource? _rebuildCts; private readonly object _rebuildLock = new(); @@ -36,6 +41,9 @@ public class SyncManager : IDisposable private long _lastEventTick; private const long EventThrottleMs = 30; + private const long ForegroundDebounceMs = 250; + private const long RebuildCooldownMs = 2000; + public SyncManager(ProfileStore store, WindowEnumerator enumerator, WinEventHookManager hookManager, ILogger log) { @@ -74,9 +82,12 @@ public void RebuildGroups() } } - _syncGroups = newGroups; - _profileNames = newNames; - _lastRebuildTick = Environment.TickCount64; + lock (_syncLock) + { + _syncGroups = newGroups; + _profileNames = newNames; + _lastRebuildTick = Environment.TickCount64; + } } /// @@ -117,9 +128,12 @@ public void UpdateHooksIfNeeded(bool skipRebuild = false) else { _hookManager.Uninstall(); - _syncGroups.Clear(); - _profileNames.Clear(); - _lastMinMaxByHwnd.Clear(); + lock (_syncLock) + { + _syncGroups.Clear(); + _profileNames.Clear(); + _lastMinMaxByHwnd.Clear(); + } } } @@ -152,9 +166,12 @@ private void OnWinEvent(uint eventType, nint hwnd) if (!NativeMethods.IsWindow(hwnd)) return; var mm = WindowEnumerator.GetMinMax(hwnd); - if (_lastMinMaxByHwnd.TryGetValue(hwnd, out var prev) && prev == mm) - return; - _lastMinMaxByHwnd[hwnd] = mm; + lock (_syncLock) + { + if (_lastMinMaxByHwnd.TryGetValue(hwnd, out var prev) && prev == mm) + return; + _lastMinMaxByHwnd[hwnd] = mm; + } var groups = GetGroupsContainingHwnd(hwnd); if (groups.Count == 0) @@ -239,7 +256,7 @@ private void PropagateMinMax(string profileId, HashSet group, nint sourceH { NativeMethods.ShowWindow(target, NativeMethods.SW_RESTORE); } - _lastMinMaxByHwnd[target] = mm; + lock (_syncLock) { _lastMinMaxByHwnd[target] = mm; } count++; } catch (Exception ex) { _log.Debug(ex, "PropagateMinMax failed for target {Target}", target); } @@ -254,14 +271,17 @@ private void PropagateMinMax(string profileId, HashSet group, nint sourceH private void PropagateForeground(string profileId, HashSet group, nint sourceHwnd) { var now = Environment.TickCount64; - if (_lastForegroundTickByProfile.TryGetValue(profileId, out var lastTick) && - now - lastTick < 250 && - _lastForegroundByProfile.TryGetValue(profileId, out var lastHwnd) && - lastHwnd == sourceHwnd) - return; + lock (_syncLock) + { + if (_lastForegroundTickByProfile.TryGetValue(profileId, out var lastTick) && + now - lastTick < ForegroundDebounceMs && + _lastForegroundByProfile.TryGetValue(profileId, out var lastHwnd) && + lastHwnd == sourceHwnd) + return; - _lastForegroundTickByProfile[profileId] = now; - _lastForegroundByProfile[profileId] = sourceHwnd; + _lastForegroundTickByProfile[profileId] = now; + _lastForegroundByProfile[profileId] = sourceHwnd; + } int count = 0; foreach (var target in group) @@ -287,18 +307,21 @@ private void PropagateForeground(string profileId, HashSet group, nint sou private List<(string Id, HashSet Group)> GetGroupsContainingHwnd(nint hwnd) { - var result = new List<(string, HashSet)>(); - foreach (var (id, group) in _syncGroups) + lock (_syncLock) { - if (group.Contains(hwnd)) - result.Add((id, group)); + var result = new List<(string, HashSet)>(); + foreach (var (id, group) in _syncGroups) + { + if (group.Contains(hwnd)) + result.Add((id, group)); + } + return result; } - return result; } private void TryRebuild() { - if (Environment.TickCount64 - _lastRebuildTick > 2000) + if (Environment.TickCount64 - _lastRebuildTick > RebuildCooldownMs) ScheduleRebuild(50); } diff --git a/src/WindowController.Core/AppSettingsStore.cs b/src/WindowController.Core/AppSettingsStore.cs index 250198d..d7a63fb 100644 --- a/src/WindowController.Core/AppSettingsStore.cs +++ b/src/WindowController.Core/AppSettingsStore.cs @@ -15,8 +15,7 @@ public class AppSettingsStore private static readonly JsonSerializerOptions JsonOptions = new() { WriteIndented = true, - DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull, - Encoder = System.Text.Encodings.Web.JavaScriptEncoder.UnsafeRelaxedJsonEscaping + DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull }; private readonly string _filePath; diff --git a/src/WindowController.Core/PathNormalizer.cs b/src/WindowController.Core/PathNormalizer.cs index 09cee1f..8d225a9 100644 --- a/src/WindowController.Core/PathNormalizer.cs +++ b/src/WindowController.Core/PathNormalizer.cs @@ -13,20 +13,44 @@ public static string Normalize(string? path) // Drive path: C:\\\\foo -> C:\foo if (path.Length >= 3 && char.IsLetter(path[0]) && path[1] == ':' && path[2] == '\\') { - while (path.Contains("\\\\")) - path = path.Replace("\\\\", "\\"); - return path; + return NormalizeBackslashes(path); } // UNC: \\\\server\\share -> \\server\share if (path.StartsWith("\\\\")) { var tail = path.TrimStart('\\'); - while (tail.Contains("\\\\")) - tail = tail.Replace("\\\\", "\\"); - return "\\\\" + tail; + return "\\\\" + NormalizeBackslashes(tail); } return path; } + + /// + /// Replace consecutive backslashes with a single backslash in one pass. + /// + private static string NormalizeBackslashes(string value) + { + // Fast-path: no double backslash present + if (!value.Contains("\\\\")) + return value; + + var sb = new System.Text.StringBuilder(value.Length); + bool prevWasBackslash = false; + foreach (var ch in value) + { + if (ch == '\\') + { + if (!prevWasBackslash) + sb.Append(ch); + prevWasBackslash = true; + } + else + { + sb.Append(ch); + prevWasBackslash = false; + } + } + return sb.ToString(); + } } diff --git a/src/WindowController.Core/ProfileStore.cs b/src/WindowController.Core/ProfileStore.cs index 05224e9..a4edf01 100644 --- a/src/WindowController.Core/ProfileStore.cs +++ b/src/WindowController.Core/ProfileStore.cs @@ -14,8 +14,7 @@ public class ProfileStore { WriteIndented = true, DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull, - PropertyNamingPolicy = null, // respect JsonPropertyName attributes - Encoder = System.Text.Encodings.Web.JavaScriptEncoder.UnsafeRelaxedJsonEscaping + PropertyNamingPolicy = null // respect JsonPropertyName attributes }; private string _filePath; @@ -62,12 +61,12 @@ public void Load() { var json = File.ReadAllText(_filePath); var parsed = JsonSerializer.Deserialize(json, JsonOptions); - + // Detect migration need before normalization - bool needsMigration = parsed != null && parsed.Profiles.Any(p => - string.IsNullOrEmpty(p.Id) || + bool needsMigration = parsed != null && parsed.Profiles.Any(p => + string.IsNullOrEmpty(p.Id) || parsed.Profiles.Count(x => x.Id == p.Id) > 1); - + var normalized = NormalizeData(parsed ?? new ProfilesRoot()); Data = normalized; @@ -156,11 +155,11 @@ public bool DeleteProfileById(string id) var trimmed = desiredName.Trim(); var finalName = ResolveUniqueName(trimmed, profileId); - + // Skip save if the name hasn't actually changed if (finalName == profile.Name) return finalName; - + profile.Name = finalName; profile.UpdatedAt = DateTime.Now.ToString("yyyy-MM-dd'T'HH:mm:ss"); Save(); diff --git a/src/WindowController.Win32/VirtualDesktopMoveHelper.cs b/src/WindowController.Win32/VirtualDesktopMoveHelper.cs index 734cd48..bf58538 100644 --- a/src/WindowController.Win32/VirtualDesktopMoveHelper.cs +++ b/src/WindowController.Win32/VirtualDesktopMoveHelper.cs @@ -113,6 +113,11 @@ private interface IVdmInternal_22H2 // ── Public API ── + /// + /// Maximum time to wait for the MTA thread to complete COM operations. + /// + private static readonly TimeSpan MtaThreadTimeout = TimeSpan.FromSeconds(5); + /// /// Move a window to the specified virtual desktop using undocumented internal COM. /// Tries each known build config; returns true on the first successful move. @@ -147,7 +152,7 @@ public static bool TryMoveWindowToDesktop(nint hwnd, Guid desktopId, ILogger log thread.Start(); // Avoid blocking indefinitely in case the shell COM call hangs. - var completed = thread.Join(TimeSpan.FromSeconds(5)); + var completed = thread.Join(MtaThreadTimeout); if (!completed) { log.Warning("VDMoveHelper: MTA thread did not complete within the timeout; aborting move"); @@ -255,7 +260,7 @@ private static bool TryWith24H2(IServiceProvider10 sp, nint pView, Guid desktopI return false; log.Debug("VDMoveHelper: matched 24H2 interface"); - return DoMove(vdm, pView, desktopId, log); + return DoMove(vdm.FindDesktop, vdm.MoveViewToDesktop, pView, desktopId, log); } catch (InvalidCastException) { @@ -281,7 +286,7 @@ private static bool TryWith22H2(IServiceProvider10 sp, nint pView, Guid desktopI return false; log.Debug("VDMoveHelper: matched 22H2 interface"); - return DoMove(vdm, pView, desktopId, log); + return DoMove(vdm.FindDesktop, vdm.MoveViewToDesktop, pView, desktopId, log); } catch (InvalidCastException) { @@ -293,50 +298,28 @@ private static bool TryWith22H2(IServiceProvider10 sp, nint pView, Guid desktopI } } + // Delegates matching the COM method signatures, shared between build-specific interfaces. + private delegate int FindDesktopFunc(ref Guid desktopId, out nint ppDesktop); + private delegate int MoveViewFunc(nint pView, nint pDesktop); + /// - /// FindDesktop + MoveViewToDesktop — generic over both build interfaces. + /// FindDesktop + MoveViewToDesktop — generic over both build interfaces + /// via delegates to avoid duplicating the same logic. /// - private static bool DoMove(IVdmInternal_24H2 vdm, nint pView, Guid desktopId, ILogger log) - { - nint pDesktop = 0; - try - { - int hr = vdm.FindDesktop(ref desktopId, out pDesktop); - if (hr != 0 || pDesktop == 0) - { - log.Debug("VDMoveHelper: FindDesktop hr=0x{Hr:X8}", hr); - return false; - } - - hr = vdm.MoveViewToDesktop(pView, pDesktop); - if (hr != 0) - { - log.Debug("VDMoveHelper: MoveViewToDesktop hr=0x{Hr:X8}", hr); - return false; - } - - log.Debug("VDMoveHelper: moved window to desktop {Desktop}", desktopId); - return true; - } - finally - { - if (pDesktop != 0) Marshal.Release(pDesktop); - } - } - - private static bool DoMove(IVdmInternal_22H2 vdm, nint pView, Guid desktopId, ILogger log) + private static bool DoMove(FindDesktopFunc findDesktop, MoveViewFunc moveView, + nint pView, Guid desktopId, ILogger log) { nint pDesktop = 0; try { - int hr = vdm.FindDesktop(ref desktopId, out pDesktop); + int hr = findDesktop(ref desktopId, out pDesktop); if (hr != 0 || pDesktop == 0) { log.Debug("VDMoveHelper: FindDesktop hr=0x{Hr:X8}", hr); return false; } - hr = vdm.MoveViewToDesktop(pView, pDesktop); + hr = moveView(pView, pDesktop); if (hr != 0) { log.Debug("VDMoveHelper: MoveViewToDesktop hr=0x{Hr:X8}", hr); diff --git a/src/WindowController.Win32/WindowArranger.cs b/src/WindowController.Win32/WindowArranger.cs index 772d121..c392339 100644 --- a/src/WindowController.Win32/WindowArranger.cs +++ b/src/WindowController.Win32/WindowArranger.cs @@ -191,11 +191,13 @@ public ArrangeResult Arrange(nint hwnd, WindowEntry entry, MonitorData? forceMon private void ApplyRect(nint hwnd, int x, int y, int w, int h, int targetState) { + const int RestoreSettleMs = 30; + try { // Restore first to allow positioning NativeMethods.ShowWindow(hwnd, NativeMethods.SW_RESTORE); - Thread.Sleep(30); + Thread.Sleep(RestoreSettleMs); // Set position and size NativeMethods.SetWindowPos(hwnd, 0, x, y, w, h, NativeMethods.SWP_NOZORDER | NativeMethods.SWP_NOACTIVATE); @@ -222,11 +224,11 @@ private void ApplyRect(nint hwnd, int x, int y, int w, int h, int targetState) private static void Clamp(ref int x, ref int y, ref int w, ref int h, WorkArea wa) { - const int MinVisible = 100; + const int MinVisibleSize = 100; // Ensure minimum size - if (w < MinVisible) w = MinVisible; - if (h < MinVisible) h = MinVisible; + if (w < MinVisibleSize) w = MinVisibleSize; + if (h < MinVisibleSize) h = MinVisibleSize; // Clamp size to work area + margin (DWM border can exceed work area) int maxW = wa.Width + 2 * DwmFrameMargin; diff --git a/src/WindowController.Win32/WindowEnumerator.cs b/src/WindowController.Win32/WindowEnumerator.cs index 7e56577..2b1d71b 100644 --- a/src/WindowController.Win32/WindowEnumerator.cs +++ b/src/WindowController.Win32/WindowEnumerator.cs @@ -36,6 +36,7 @@ public class WindowEnumerator // WMI command-line cache: PID → (commandLine, tickWhenCached) private readonly ConcurrentDictionary _cmdLineCache = new(); private const long CmdLineCacheTtlMs = 60_000; // 1 minute + private const int CmdLineCacheMaxEntries = 200; public WindowEnumerator(ILogger logger, Func? urlGetter = null) { @@ -259,7 +260,7 @@ public string GetCommandLineCached(uint pid) _cmdLineCache[pid] = (cmdLine, now); // Prune stale entries periodically - if (_cmdLineCache.Count > 200) + if (_cmdLineCache.Count > CmdLineCacheMaxEntries) { var staleKeys = _cmdLineCache .Where(kv => now - kv.Value.Tick > CmdLineCacheTtlMs * 2)