1444511 - IsRemoteFilePath in nsLocalFileWin.cpp may be incorrect

Status: NEW

(Core :: XPCOM, enhancement)

Description

[Dana Keeler (she/her) (use needinfo) [:keeler]]

static bool
IsRemoteFilePath(LPCWSTR aPath, bool& aRemote)
{
  // Obtain the parent directory path and make sure it ends with
  // a trailing backslash.
  WCHAR dirPath[MAX_PATH + 1] = { 0 };
  wcsncpy(dirPath, aPath, MAX_PATH);
  if (!PathRemoveFileSpecW(dirPath)) {
    return false;
  }
  size_t len = wcslen(dirPath);
  // In case the dirPath holds exaclty MAX_PATH and remains unchanged, we
  // recheck the required length here since we need to terminate it with
  // a backslash.
  if (len >= MAX_PATH) {
    return false;
  }

  dirPath[len] = L'\\';
  dirPath[len + 1] = L'\0';
  UINT driveType = GetDriveTypeW(dirPath);
  aRemote = driveType == DRIVE_REMOTE;
  return true;
}

From my reading of the documentation, GetDriveTypeW requires a path describing the root directory of a drive [0]. However, PathRemoveFileSpecW only removes the trailing filename from a path [1]. Consequently, what's getting passed to GetDriveTypeW may not be guaranteed to be the root directory of a drive. I believe a correct way would be to call GetVolumePathNameW instead [2]. Note that this is already what GMPProcessParent::Launch does [3]. Bug 1435376 may also be adding another instance of this pattern shortly.

[0] https://msdn.microsoft.com/en-us/library/windows/desktop/aa364939(v=vs.85).aspx
[1] https://msdn.microsoft.com/en-us/library/windows/desktop/bb773748(v=vs.85).aspx
[2] https://msdn.microsoft.com/en-us/library/windows/desktop/aa364996(v=vs.85).aspx
[3] https://searchfox.org/mozilla-central/rev/588d8120aa11738657da93e09a03378bcd1ba8ec/dom/media/gmp/GMPProcessParent.cpp#65