7.9

1 files changed, 31 insertions, 25 deletions

diff --git a/zen/dir_watcher.cpp b/zen/dir_watcher.cpp
index 97ecafe5..e7748519 100644
--- a/zen/dir_watcher.cpp
+++ b/zen/dir_watcher.cpp
@@ -39,7 +39,7 @@ class SharedData
 {
 public:
     //context of worker thread
-    void addChanges(const char* buffer, DWORD bytesWritten, const Zstring& dirpath) //throw ()
+    bool tryAddChanges(const char* buffer, DWORD bytesWritten, const Zstring& dirPathPf) //noexcept; return false on failure (already reported!)
     {
         std::lock_guard<std::mutex> dummy(lockAccess);
 
@@ -47,12 +47,20 @@ public:
             changedFiles.emplace_back(DirWatcher::ACTION_CREATE, L"Overflow.");
         else
         {
-            const char* bufPos = &buffer[0];
+            size_t offset = 0;
             for (;;)
             {
-                const FILE_NOTIFY_INFORMATION& notifyInfo = reinterpret_cast<const FILE_NOTIFY_INFORMATION&>(*bufPos);
+                const FILE_NOTIFY_INFORMATION& notifyInfo = *reinterpret_cast<const FILE_NOTIFY_INFORMATION*>(buffer + offset);
 
-                const Zstring fullpath = dirpath + Zstring(notifyInfo.FileName, notifyInfo.FileNameLength / sizeof(WCHAR));
+                //access-violation crash dumps suggest that the buffer can be corrupt, so let's validate:
+                if (offset + sizeof(FILE_NOTIFY_INFORMATION) > bytesWritten ||
+                    offset + offsetof(FILE_NOTIFY_INFORMATION, FileName) + notifyInfo.FileNameLength > bytesWritten)
+                {
+                    reportError(replaceCpy(_("Cannot monitor directory %x."), L"%x", fmtPath(dirPathPf)), L"ReadDirectoryChangesW: corrupt data returned");
+                    return false;
+                }
+
+                const Zstring fullpath = dirPathPf + Zstring(notifyInfo.FileName, notifyInfo.FileNameLength / sizeof(WCHAR));
 
                 [&]
                 {
@@ -85,16 +93,17 @@ public:
 
                 if (notifyInfo.NextEntryOffset == 0)
                     break;
-                bufPos += notifyInfo.NextEntryOffset;
+                offset += notifyInfo.NextEntryOffset;
             }
         }
+        return true;
     }
 
     ////context of main thread
-    //void addChange(const Zstring& dirpath) //throw ()
+    //void addChange(const Zstring& dirPath) //throw ()
     //{
     //    std::lock_guard<std::mutex> dummy(lockAccess);
-    //    changedFiles.insert(dirpath);
+    //    changedFiles.insert(dirPath);
     //}
 
 
@@ -105,11 +114,8 @@ public:
 
         //first check whether errors occurred in thread
         if (errorInfo)
-        {
-            const std::wstring msg   = copyStringTo<std::wstring>(errorInfo->msg);
-            const std::wstring descr = copyStringTo<std::wstring>(errorInfo->descr);
-            throw FileError(msg, descr);
-        }
+            throw FileError(copyStringTo<std::wstring>(errorInfo->msg),
+                            copyStringTo<std::wstring>(errorInfo->descr));
 
         output.swap(changedFiles);
         changedFiles.clear();
@@ -117,10 +123,10 @@ public:
 
 
     //context of worker thread
-    void reportError(const std::wstring& msg, const std::wstring& description, DWORD errorCode) //throw()
+    void reportError(const std::wstring& msg, const std::wstring& description) //throw()
     {
         std::lock_guard<std::mutex> dummy(lockAccess);
-        errorInfo = ErrorInfo({ copyStringTo<BasicWString>(msg), copyStringTo<BasicWString>(description), errorCode });
+        errorInfo = ErrorInfo({ copyStringTo<BasicWString>(msg), copyStringTo<BasicWString>(description) });
     }
 
 private:
@@ -133,7 +139,6 @@ private:
     {
         BasicWString msg;
         BasicWString descr;
-        DWORD errorCode;
     };
     Opt<ErrorInfo> errorInfo; //non-empty if errors occurred in thread
 };
@@ -146,9 +151,9 @@ public:
     ReadChangesAsync(const Zstring& directory, //make sure to not leak-in thread-unsafe types!
                      const std::shared_ptr<SharedData>& shared) :
         shared_(shared),
-        dirpathPf(appendSeparator(directory))        
+        dirPathPf(appendSeparator(directory))
     {
-        hDir = ::CreateFile(applyLongPathPrefix(dirpathPf).c_str(),                 //_In_      LPCTSTR lpFileName,
+        hDir = ::CreateFile(applyLongPathPrefix(dirPathPf).c_str(),                 //_In_      LPCTSTR lpFileName,
                             FILE_LIST_DIRECTORY,                                    //_In_      DWORD dwDesiredAccess,
                             FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, //_In_      DWORD dwShareMode,
                             nullptr,              //_In_opt_  LPSECURITY_ATTRIBUTES lpSecurityAttributes,
@@ -163,7 +168,7 @@ public:
     }
 
     ReadChangesAsync(ReadChangesAsync&& other) : shared_(std::move(other.shared_)),
-        dirpathPf(std::move(other.dirpathPf)),
+        dirPathPf(std::move(other.dirPathPf)),
         hDir(other.hDir)
     {
         other.hDir = INVALID_HANDLE_VALUE;
@@ -192,7 +197,7 @@ public:
             if (overlapped.hEvent == nullptr)
             {
                 const DWORD ec = ::GetLastError(); //copy before directly/indirectly making other system calls!
-                return shared_->reportError(replaceCpy(_("Cannot monitor directory %x."), L"%x", fmtPath(dirpathPf)), formatSystemError(L"CreateEvent", ec), ec);
+                return shared_->reportError(replaceCpy(_("Cannot monitor directory %x."), L"%x", fmtPath(dirPathPf)), formatSystemError(L"CreateEvent", ec));
             }
             ZEN_ON_SCOPE_EXIT(::CloseHandle(overlapped.hEvent));
 
@@ -212,7 +217,7 @@ public:
                                          nullptr))                      //  __in_opt     LPOVERLAPPED_COMPLETION_ROUTINE lpCompletionRoutine
             {
                 const DWORD ec = ::GetLastError(); //copy before directly/indirectly making other system calls!
-                return shared_->reportError(replaceCpy(_("Cannot monitor directory %x."), L"%x", fmtPath(dirpathPf)), formatSystemError(L"ReadDirectoryChangesW", ec), ec);
+                return shared_->reportError(replaceCpy(_("Cannot monitor directory %x."), L"%x", fmtPath(dirPathPf)), formatSystemError(L"ReadDirectoryChangesW", ec));
             }
 
             //async I/O is a resource that needs to be guarded since it will write to local variable "buffer"!
@@ -239,7 +244,7 @@ public:
             {
                 const DWORD ec = ::GetLastError(); //copy before directly/indirectly making other system calls!
                 if (ec != ERROR_IO_INCOMPLETE)
-                    return shared_->reportError(replaceCpy(_("Cannot monitor directory %x."), L"%x", fmtPath(dirpathPf)), formatSystemError(L"GetOverlappedResult", ec), ec);
+                    return shared_->reportError(replaceCpy(_("Cannot monitor directory %x."), L"%x", fmtPath(dirPathPf)), formatSystemError(L"GetOverlappedResult", ec));
 
                 //execute asynchronous procedure calls (APC) queued on this thread
                 ::SleepEx(50,    // __in  DWORD dwMilliseconds,
@@ -249,7 +254,8 @@ public:
             }
             guardAio.dismiss();
 
-            shared_->addChanges(&buffer[0], bytesWritten, dirpathPf); //throw ()
+            if (!shared_->tryAddChanges(&buffer[0], bytesWritten, dirPathPf)) //noexcept
+                return;
         }
     }
 
@@ -262,7 +268,7 @@ private:
     //shared between main and worker:
     std::shared_ptr<SharedData> shared_;
     //worker thread only:
-    Zstring dirpathPf; //thread safe!
+    Zstring dirPathPf; //thread safe!
     HANDLE hDir = INVALID_HANDLE_VALUE;
 };
 
@@ -275,7 +281,7 @@ public:
                         InterruptibleThread& worker) :
         notificationHandle(registerFolderRemovalNotification(hDir, //throw FileError
                                                              displayPath,
-                                                             [this]{ this->onRequestRemoval (); }, //noexcept!
+                                                             [this] { this->onRequestRemoval (); }, //noexcept!
     [this](bool successful) { this->onRemovalFinished(); })), //
     worker_(worker) {}
 
@@ -549,7 +555,7 @@ void eventCallback(ConstFSEventStreamRef streamRef,
 
 struct DirWatcher::Pimpl
 {
-	FSEventStreamRef eventStream = nullptr;
+    FSEventStreamRef eventStream = nullptr;
     std::vector<DirWatcher::Entry> changedFiles;
 };