Bug 1287426 Part 6: Re-apply - Change Chromium sandbox to allow rules for files on network drives to be added. r=aklotz
authorBob Owen <bobowencode@gmail.com>
Mon, 01 Feb 2016 08:59:00 +0000
changeset 354090 9069b89a690b7d65dc1fd3e00cc80f6e1774ecb1
parent 354089 a05726163a79b404bf3248a86e3261e44cf50793
child 354091 e834e810a3faaa0edf7c364677563589e39f5098
push id6570
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:26:13 +0000
treeherdermozilla-beta@f455459b2ae5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaklotz
bugs1287426
milestone51.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1287426 Part 6: Re-apply - Change Chromium sandbox to allow rules for files on network drives to be added. r=aklotz Originally landed as changset: https://hg.mozilla.org/mozilla-central/rev/afa4f68de47c MozReview-Commit-ID: A18C0KcEqvP
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,66 @@ 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)) {
+    return ERROR_INVALID_NAME;
+  }
+  size_t vol_path_len = wcslen(vol_path);
+  if (!EqualPath(path, vol_path, vol_path_len)) {
+    return ERROR_INVALID_NAME;
+  }
+
+  // vol_path will include a trailing slash, so reduce size for loop check.
+  --vol_path_len;
 
   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_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 +223,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,8 +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://hg.mozilla.org/mozilla-central/rev/afa4f68de47c
+https://bugzilla.mozilla.org/show_bug.cgi?id=1287426 bug1287426part6.patch
 https://bugzilla.mozilla.org/show_bug.cgi?id=1273852