win: fix race in uv_fs_event_start() (#4948)

Fetch file metadata by handle instead of by path, otherwise there is a
race window between fetching and acting on said metadata where another
process can replace the file with another one.

There is still a potential race when upgrading the short path to a long
path but that that one is intrinsic. It's not something libuv can solve
except by refraining from calling GetLongPathName.

Fixes: https://github.com/libuv/libuv/issues/4568

src/win/fs-event.c

@@ -158,14 +158,14 @@ int uv_fs_event_start(uv_fs_event_t* handle,
                       const char* path,
                       unsigned int flags) {
   int is_path_dir;
-  size_t size;
+  DWORD last_error;
-  DWORD attr, last_error;
+  WCHAR* dir, *pathw = NULL;
-  WCHAR* dir = NULL, *dir_to_watch, *pathw = NULL;
   DWORD short_path_buffer_len;
   WCHAR *short_path_buffer;
-  WCHAR* short_path, *long_path;
+  WCHAR* short_path = NULL;
+  HANDLE file_handle = INVALID_HANDLE_VALUE;
+  BY_HANDLE_FILE_INFORMATION info;
 
-  short_path = NULL;
   if (uv__is_active(handle))
     return UV_EINVAL;
 
@@ -181,43 +181,26 @@ int uv_fs_event_start(uv_fs_event_t* handle,
   if (last_error)
     goto error_uv;
 
-  /* Determine whether path is a file or a directory. */
+  file_handle = CreateFileW(pathw,
-  attr = GetFileAttributesW(pathw);
+                            FILE_LIST_DIRECTORY,
-  if (attr == INVALID_FILE_ATTRIBUTES) {
+                            FILE_SHARE_READ|FILE_SHARE_DELETE|FILE_SHARE_WRITE,
+                            NULL,
+                            OPEN_EXISTING,
+                            FILE_FLAG_BACKUP_SEMANTICS|FILE_FLAG_OVERLAPPED,
+                            NULL);
+  if (file_handle == INVALID_HANDLE_VALUE) {
     last_error = GetLastError();
     goto error;
   }
 
-  is_path_dir = (attr & FILE_ATTRIBUTE_DIRECTORY) ? 1 : 0;
+  if (!GetFileInformationByHandle(file_handle, &info)) {
+    last_error = GetLastError();
+    goto error;
+  }
 
-  if (is_path_dir) {
+  is_path_dir = (info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) ? 1 : 0;
-     /* path is a directory, so that's the directory that we will watch. */
 
-    /* Convert to long path. */
+  if (!is_path_dir) {
-    size = GetLongPathNameW(pathw, NULL, 0);
-
-    if (size) {
-      long_path = (WCHAR*)uv__malloc(size * sizeof(WCHAR));
-      if (!long_path) {
-        uv_fatal_error(ERROR_OUTOFMEMORY, "uv__malloc");
-      }
-
-      size = GetLongPathNameW(pathw, long_path, size);
-      if (size) {
-        long_path[size] = '\0';
-      } else {
-        uv__free(long_path);
-        long_path = NULL;
-      }
-
-      if (long_path) {
-        uv__free(pathw);
-        pathw = long_path;
-      }
-    }
-
-    dir_to_watch = pathw;
-  } else {
     /*
      * path is a file.  So we split path into dir & file parts, and
      * watch the dir directory.
@@ -252,28 +235,50 @@ short_path_done:
       goto error;
     }
 
-    dir_to_watch = dir;
     uv__free(short_path);
     short_path = NULL;
     uv__free(pathw);
     pathw = NULL;
-  }
 
-  handle->dir_handle = CreateFileW(dir_to_watch,
+    /* Open the containing directory and watch that instead. Events for
-                                   FILE_LIST_DIRECTORY,
+     * other files are filtered out in uv__process_fs_event_req().
-                                   FILE_SHARE_READ | FILE_SHARE_DELETE |
+     * Not super efficient but c'est ça.
-                                     FILE_SHARE_WRITE,
+     */
-                                   NULL,
+    CloseHandle(file_handle);
-                                   OPEN_EXISTING,
+    file_handle = CreateFileW(dir,
-                                   FILE_FLAG_BACKUP_SEMANTICS |
+                              FILE_LIST_DIRECTORY,
-                                     FILE_FLAG_OVERLAPPED,
+                              FILE_SHARE_READ|FILE_SHARE_DELETE|FILE_SHARE_WRITE,
-                                   NULL);
+                              NULL,
-
+                              OPEN_EXISTING,
-  if (dir) {
+                              FILE_FLAG_BACKUP_SEMANTICS|FILE_FLAG_OVERLAPPED,
+                              NULL);
     uv__free(dir);
     dir = NULL;
+    if (file_handle == INVALID_HANDLE_VALUE) {
+      last_error = GetLastError();
+      goto error;
+    }
+
+    if (!GetFileInformationByHandle(file_handle, &info)) {
+      last_error = GetLastError();
+      goto error;
+    }
+
+    /* Race with another process: directory foo in foo/bar was replaced
+     * with a file. Bail out with an error, we're not recursing upwards.
+     */
+    if (!(info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)) {
+      /* TODO(bnoordhuis) ERROR_DIRECTORY is translated to UV_ENOENT,
+       * there's currently nothing that maps to UV_ENOTDIR.
+       */
+      last_error = ERROR_DIRECTORY;
+      goto error;
+    }
   }
 
+  handle->dir_handle = file_handle;
+  file_handle = INVALID_HANDLE_VALUE;
+
   if (handle->dir_handle == INVALID_HANDLE_VALUE) {
     last_error = GetLastError();
     goto error;
@@ -325,6 +330,11 @@ error:
   last_error = uv_translate_sys_error(last_error);
 
 error_uv:
+  if (file_handle != INVALID_HANDLE_VALUE) {
+    CloseHandle(file_handle);
+    file_handle = INVALID_HANDLE_VALUE;
+  }
+
   if (handle->path) {
     uv__free(handle->path);
     handle->path = NULL;