Bug 1273372 Part 2: Re-apply change to allow network drives in sandbox rules with non-file device fix. r=aklotz a=lizzard
authorBob Owen <bobowencode@gmail.com>
Mon, 01 Feb 2016 08:59:00 +0000
changeset 359250 00780d8c7b9421d4d3e714dc9b00cbeb469c9c41
parent 359249 5339b68d8a2a96e1f6c90544fc32e1408536671e
child 359251 a9402f818972a194d452af23452744a9441e077e
push id1324
push usermtabara@mozilla.com
push dateMon, 16 Jan 2017 13:07:44 +0000
treeherdermozilla-release@a01c49833940 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaklotz, lizzard
bugs1273372
milestone51.0
Bug 1273372 Part 2: Re-apply change to allow network drives in sandbox rules with non-file device fix. r=aklotz a=lizzard
security/sandbox/chromium/sandbox/win/src/win_utils.cc
security/sandbox/modifications-to-chromium-to-reapply-after-upstream-merge.txt
--- a/security/sandbox/chromium/sandbox/win/src/win_utils.cc
+++ b/security/sandbox/chromium/sandbox/win/src/win_utils.cc
@@ -154,63 +154,68 @@ bool ResolveRegistryName(base::string16 
 
   return false;
 }
 
 // |full_path| can have any of the following forms:
 //    \??\c:\some\foo\bar
 //    \Device\HarddiskVolume0\some\foo\bar
 //    \??\HarddiskVolume0\some\foo\bar
+//    \??\UNC\SERVER\Share\some\foo\bar
 DWORD IsReparsePoint(const base::string16& full_path) {
   // Check if it's a pipe. We can't query the attributes of a pipe.
   if (IsPipe(full_path))
     return ERROR_NOT_A_REPARSE_POINT;
 
   base::string16 path;
   bool nt_path = IsNTPath(full_path, &path);
   bool has_drive = StartsWithDriveLetter(path);
   bool is_device_path = IsDevicePath(path, &path);
 
   if (!has_drive && !is_device_path && !nt_path)
     return ERROR_INVALID_NAME;
 
-  bool added_implied_device = false;
   if (!has_drive) {
-      path = base::string16(kNTDotPrefix) + path;
-      added_implied_device = true;
+    // Add Win32 device namespace prefix, required for some Windows APIs.
+    path.insert(0, kNTDotPrefix);
   }
 
-  base::string16::size_type last_pos = base::string16::npos;
-  bool passed_once = false;
+  // Ensure that volume path matches start of path.
+  wchar_t vol_path[MAX_PATH];
+  if (!::GetVolumePathNameW(path.c_str(), vol_path, MAX_PATH)) {
+    // This will fail if this is a device that isn't volume related, which can't
+    // then be a reparse point.
+    return is_device_path ? ERROR_NOT_A_REPARSE_POINT : ERROR_INVALID_NAME;
+  }
+
+  // vol_path includes a trailing slash, so reduce size for path and loop check.
+  size_t vol_path_len = wcslen(vol_path) - 1;
+  if (!EqualPath(path, vol_path, vol_path_len)) {
+    return ERROR_INVALID_NAME;
+  }
 
   do {
-    path = path.substr(0, last_pos);
-
     DWORD attributes = ::GetFileAttributes(path.c_str());
     if (INVALID_FILE_ATTRIBUTES == attributes) {
       DWORD error = ::GetLastError();
       if (error != ERROR_FILE_NOT_FOUND &&
           error != ERROR_PATH_NOT_FOUND &&
+          error != ERROR_INVALID_FUNCTION &&
           error != ERROR_INVALID_NAME) {
         // Unexpected error.
-        if (passed_once && added_implied_device &&
-            (path.rfind(L'\\') == kNTDotPrefixLen - 1)) {
-          break;
-        }
         NOTREACHED_NT();
         return error;
       }
     } else if (FILE_ATTRIBUTE_REPARSE_POINT & attributes) {
       // This is a reparse point.
       return ERROR_SUCCESS;
     }
 
-    passed_once = true;
-    last_pos = path.rfind(L'\\');
-  } while (last_pos > 2);  // Skip root dir.
+    path.resize(path.rfind(L'\\'));
+  } while (path.size() > vol_path_len);  // Skip root dir.
 
   return ERROR_NOT_A_REPARSE_POINT;
 }
 
 // We get a |full_path| of the forms accepted by IsReparsePoint(), and the name
 // we'll get from |handle| will be \device\harddiskvolume1\some\foo\bar.
 bool SameObject(HANDLE handle, const wchar_t* full_path) {
   // Check if it's a pipe.
@@ -220,63 +225,67 @@ bool SameObject(HANDLE handle, const wch
   base::string16 actual_path;
   if (!GetPathFromHandle(handle, &actual_path))
     return false;
 
   base::string16 path(full_path);
   DCHECK_NT(!path.empty());
 
   // This may end with a backslash.
-  const wchar_t kBackslash = '\\';
-  if (path[path.length() - 1] == kBackslash)
-    path = path.substr(0, path.length() - 1);
+  if (path.back() == L'\\') {
+    path.pop_back();
+  }
 
-  // Perfect match (case-insesitive check).
+  // Perfect match (case-insensitive check).
   if (EqualPath(actual_path, path))
     return true;
 
   bool nt_path = IsNTPath(path, &path);
   bool has_drive = StartsWithDriveLetter(path);
 
   if (!has_drive && nt_path) {
     base::string16 simple_actual_path;
-    if (!IsDevicePath(actual_path, &simple_actual_path))
-      return false;
-
-    // Perfect match (case-insesitive check).
-    return (EqualPath(simple_actual_path, path));
+    if (IsDevicePath(path, &path)) {
+      if (IsDevicePath(actual_path, &simple_actual_path)) {
+        // Perfect match (case-insensitive check).
+        return (EqualPath(simple_actual_path, path));
+      } else {
+        return false;
+      }
+    } else {
+      // Add Win32 device namespace for GetVolumePathName.
+      path.insert(0, kNTDotPrefix);
+    }
   }
 
-  if (!has_drive)
+  // Get the volume path in the same format as actual_path.
+  wchar_t vol_path[MAX_PATH];
+  if (!::GetVolumePathName(path.c_str(), vol_path, MAX_PATH)) {
     return false;
-
-  // We only need 3 chars, but let's alloc a buffer for four.
-  wchar_t drive[4] = {0};
-  wchar_t vol_name[MAX_PATH];
-  memcpy(drive, &path[0], 2 * sizeof(*drive));
-
-  // We'll get a double null terminated string.
-  DWORD vol_length = ::QueryDosDeviceW(drive, vol_name, MAX_PATH);
-  if (vol_length < 2 || vol_length == MAX_PATH)
+  }
+  size_t vol_path_len = wcslen(vol_path);
+  base::string16 nt_vol;
+  if (!GetNtPathFromWin32Path(vol_path, &nt_vol)) {
     return false;
-
-  // Ignore the nulls at the end.
-  vol_length = static_cast<DWORD>(wcslen(vol_name));
+  }
 
   // The two paths should be the same length.
-  if (vol_length + path.size() - 2 != actual_path.size())
+  if (nt_vol.size() + path.size() - vol_path_len != actual_path.size()) {
     return false;
+  }
 
-  // Check up to the drive letter.
-  if (!EqualPath(actual_path, vol_name, vol_length))
+  // Check the volume matches.
+  if (!EqualPath(actual_path, nt_vol.c_str(), nt_vol.size())) {
     return false;
+  }
 
-  // Check the path after the drive letter.
-  if (!EqualPath(actual_path, vol_length, path, 2))
+  // Check the path after the volume matches.
+  if (!EqualPath(actual_path, nt_vol.size(), path, vol_path_len)) {
     return false;
+  }
 
   return true;
 }
 
 // Paths like \Device\HarddiskVolume0\some\foo\bar are assumed to be already
 // expanded.
 bool ConvertToLongPath(base::string16* path) {
   if (IsPipe(*path))
--- a/security/sandbox/modifications-to-chromium-to-reapply-after-upstream-merge.txt
+++ b/security/sandbox/modifications-to-chromium-to-reapply-after-upstream-merge.txt
@@ -1,7 +1,8 @@
 Please add a link to the bugzilla bug and patch name that should be re-applied.
 Also, please update any existing links to their actual mozilla-central changeset.
 
 https://bugzilla.mozilla.org/show_bug.cgi?id=1287426 bug1287426part4.patch
 https://bugzilla.mozilla.org/show_bug.cgi?id=1287426 bug1287426part5.patch
 https://hg.mozilla.org/mozilla-central/rev/7df8d6639971
 https://bugzilla.mozilla.org/show_bug.cgi?id=1287426 bug1287426part7.patch
+https://bugzilla.mozilla.org/show_bug.cgi?id=1273372 bug1273372part2.patch