From fe98a5b119670f17e070c158c9a2044187b47285 Mon Sep 17 00:00:00 2001 From: Bob Owen Date: Thu, 22 Dec 2016 11:11:06 +0000 Subject: [PATCH] Bug 1273372 Part 1: Backout change to allow network drives in sandbox rules. r=backout --- .../chromium/sandbox/win/src/win_utils.cc | 89 +++++++++---------- ...romium-to-reapply-after-upstream-merge.txt | 1 - 2 files changed, 41 insertions(+), 49 deletions(-) diff --git a/security/sandbox/chromium/sandbox/win/src/win_utils.cc b/security/sandbox/chromium/sandbox/win/src/win_utils.cc index dc8f2fe8282c..3717a971c94f 100644 --- a/security/sandbox/chromium/sandbox/win/src/win_utils.cc +++ b/security/sandbox/chromium/sandbox/win/src/win_utils.cc @@ -159,7 +159,6 @@ bool ResolveRegistryName(base::string16 name, base::string16* resolved_name) { // \??\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)) @@ -173,25 +172,18 @@ DWORD IsReparsePoint(const base::string16& full_path) { if (!has_drive && !is_device_path && !nt_path) return ERROR_INVALID_NAME; + bool added_implied_device = false; if (!has_drive) { - // Add Win32 device namespace prefix, required for some Windows APIs. - path.insert(0, kNTDotPrefix); + path = base::string16(kNTDotPrefix) + path; + added_implied_device = true; } - // 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; + base::string16::size_type last_pos = base::string16::npos; + bool passed_once = false; do { + path = path.substr(0, last_pos); + DWORD attributes = ::GetFileAttributes(path.c_str()); if (INVALID_FILE_ATTRIBUTES == attributes) { DWORD error = ::GetLastError(); @@ -199,6 +191,10 @@ DWORD IsReparsePoint(const base::string16& full_path) { 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; } @@ -207,8 +203,9 @@ DWORD IsReparsePoint(const base::string16& full_path) { return ERROR_SUCCESS; } - path.resize(path.rfind(L'\\')); - } while (path.size() > vol_path_len); // Skip root dir. + passed_once = true; + last_pos = path.rfind(L'\\'); + } while (last_pos > 2); // Skip root dir. return ERROR_NOT_A_REPARSE_POINT; } @@ -228,11 +225,11 @@ bool SameObject(HANDLE handle, const wchar_t* full_path) { DCHECK_NT(!path.empty()); // This may end with a backslash. - if (path.back() == L'\\') { - path.pop_back(); - } + const wchar_t kBackslash = '\\'; + if (path[path.length() - 1] == kBackslash) + path = path.substr(0, path.length() - 1); - // Perfect match (case-insensitive check). + // Perfect match (case-insesitive check). if (EqualPath(actual_path, path)) return true; @@ -241,44 +238,40 @@ bool SameObject(HANDLE handle, const wchar_t* full_path) { if (!has_drive && nt_path) { base::string16 simple_actual_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 (!IsDevicePath(actual_path, &simple_actual_path)) + return false; + + // Perfect match (case-insesitive check). + return (EqualPath(simple_actual_path, path)); } - // 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)) { + if (!has_drive) return false; - } - size_t vol_path_len = wcslen(vol_path); - base::string16 nt_vol; - if (!GetNtPathFromWin32Path(vol_path, &nt_vol)) { + + // 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) return false; - } + + // Ignore the nulls at the end. + vol_length = static_cast(wcslen(vol_name)); // The two paths should be the same length. - if (nt_vol.size() + path.size() - vol_path_len != actual_path.size()) { + if (vol_length + path.size() - 2 != actual_path.size()) return false; - } - // Check the volume matches. - if (!EqualPath(actual_path, nt_vol.c_str(), nt_vol.size())) { + // Check up to the drive letter. + if (!EqualPath(actual_path, vol_name, vol_length)) return false; - } - // Check the path after the volume matches. - if (!EqualPath(actual_path, nt_vol.size(), path, vol_path_len)) { + // Check the path after the drive letter. + if (!EqualPath(actual_path, vol_length, path, 2)) return false; - } return true; } diff --git a/security/sandbox/modifications-to-chromium-to-reapply-after-upstream-merge.txt b/security/sandbox/modifications-to-chromium-to-reapply-after-upstream-merge.txt index 3d095242e66b..a03d4164f503 100644 --- a/security/sandbox/modifications-to-chromium-to-reapply-after-upstream-merge.txt +++ b/security/sandbox/modifications-to-chromium-to-reapply-after-upstream-merge.txt @@ -4,5 +4,4 @@ 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 bug1287426part6.patch https://bugzilla.mozilla.org/show_bug.cgi?id=1287426 bug1287426part7.patch