diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ReadDirectoryChangesW.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ReadDirectoryChangesW.cs index 3790ca3c10ef7..95cf5d06facd7 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ReadDirectoryChangesW.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ReadDirectoryChangesW.cs @@ -28,13 +28,8 @@ internal struct FILE_NOTIFY_INFORMATION internal FileAction Action; // Note that the file name is not null terminated - private uint FileNameLength; - private char _FileName; - - internal unsafe ReadOnlySpan FileName - { - get { fixed (char* c = &_FileName) { return new ReadOnlySpan(c, (int)FileNameLength / sizeof(char)); } } - } + internal readonly uint FileNameLength; + internal readonly char FileName; } internal enum FileAction : uint diff --git a/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.Win32.cs b/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.Win32.cs index 26e2ee6308553..e2c3756f084fe 100644 --- a/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.Win32.cs +++ b/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.Win32.cs @@ -235,7 +235,7 @@ private void ReadDirectoryChangesCallback(uint errorCode, uint numBytes, AsyncRe } else { - ParseEventBufferAndNotifyForEach(state.Buffer); + ParseEventBufferAndNotifyForEach(state.Buffer, numBytes); } } finally @@ -245,13 +245,18 @@ private void ReadDirectoryChangesCallback(uint errorCode, uint numBytes, AsyncRe } } - private unsafe void ParseEventBufferAndNotifyForEach(byte[] buffer) + private unsafe void ParseEventBufferAndNotifyForEach(byte[] buffer, uint numBytes) { Debug.Assert(buffer != null); Debug.Assert(buffer.Length > 0); + Debug.Assert(numBytes <= buffer.Length); + + numBytes = Math.Min(numBytes, (uint)buffer.Length); fixed (byte* b = buffer) { + byte* pBufferLimit = b + numBytes; + Interop.Kernel32.FILE_NOTIFY_INFORMATION* info = (Interop.Kernel32.FILE_NOTIFY_INFORMATION*)b; ReadOnlySpan oldName = ReadOnlySpan.Empty; @@ -259,6 +264,22 @@ private unsafe void ParseEventBufferAndNotifyForEach(byte[] buffer) // Parse each event from the buffer and notify appropriate delegates do { + // Validate the data we received in case it's corrupted. This can happen + // if we are watching files over the network with a poor connection. + + // Verify the info object is within the expected bounds + if (info < b || (((byte*)info) + sizeof(Interop.Kernel32.FILE_NOTIFY_INFORMATION)) > pBufferLimit) + { + break; + } + + // Verify the file path is within the bounds + byte* pFileEnd = ((byte*)&(info->FileName)) + info->FileNameLength; + if (pFileEnd < &(info->FileName) || pFileEnd > pBufferLimit) + { + break; + } + // A slightly convoluted piece of code follows. Here's what's happening: // // We wish to collapse the poorly done rename notifications from the @@ -284,17 +305,19 @@ private unsafe void ParseEventBufferAndNotifyForEach(byte[] buffer) // // (Phew!) + ReadOnlySpan fileName = new ReadOnlySpan(&(info->FileName), (int)info->FileNameLength / sizeof(char)); + switch (info->Action) { case Interop.Kernel32.FileAction.FILE_ACTION_RENAMED_OLD_NAME: // Action is renamed from, save the name of the file - oldName = info->FileName; + oldName = fileName; break; case Interop.Kernel32.FileAction.FILE_ACTION_RENAMED_NEW_NAME: // oldName may be empty if we didn't receive FILE_ACTION_RENAMED_OLD_NAME first NotifyRenameEventArgs( WatcherChangeTypes.Renamed, - info->FileName, + fileName, oldName); oldName = ReadOnlySpan.Empty; break; @@ -309,13 +332,13 @@ private unsafe void ParseEventBufferAndNotifyForEach(byte[] buffer) switch (info->Action) { case Interop.Kernel32.FileAction.FILE_ACTION_ADDED: - NotifyFileSystemEventArgs(WatcherChangeTypes.Created, info->FileName); + NotifyFileSystemEventArgs(WatcherChangeTypes.Created, fileName); break; case Interop.Kernel32.FileAction.FILE_ACTION_REMOVED: - NotifyFileSystemEventArgs(WatcherChangeTypes.Deleted, info->FileName); + NotifyFileSystemEventArgs(WatcherChangeTypes.Deleted, fileName); break; case Interop.Kernel32.FileAction.FILE_ACTION_MODIFIED: - NotifyFileSystemEventArgs(WatcherChangeTypes.Changed, info->FileName); + NotifyFileSystemEventArgs(WatcherChangeTypes.Changed, fileName); break; default: Debug.Fail($"Unknown FileSystemEvent action type! Value: {info->Action}");