From 4f68f5682bb895fa928b966f2a0060f94d2a328d Mon Sep 17 00:00:00 2001 From: Bob Owen Date: Mon, 1 Feb 2016 08:59:00 +0000 Subject: [PATCH] Bug 1173371 Part 2: Change Chromium sandbox to allow rules for files on network drives to be added. a=aklotz --- .../chromium/sandbox/win/src/win_utils.cc | 89 ++++++++++--------- ...romium-to-reapply-after-upstream-merge.txt | 3 +- 2 files changed, 50 insertions(+), 42 deletions(-) diff --git a/security/sandbox/chromium/sandbox/win/src/win_utils.cc b/security/sandbox/chromium/sandbox/win/src/win_utils.cc index d2b507d3e6a..98870ac7e52 100644 --- a/security/sandbox/chromium/sandbox/win/src/win_utils.cc +++ b/security/sandbox/chromium/sandbox/win/src/win_utils.cc @@ -156,6 +156,7 @@ 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, bool* result) { // Check if it's a pipe. We can't query the attributes of a pipe. if (IsPipe(full_path)) { @@ -171,18 +172,25 @@ DWORD IsReparsePoint(const base::string16& full_path, bool* result) { 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(); @@ -190,10 +198,6 @@ DWORD IsReparsePoint(const base::string16& full_path, bool* result) { 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; } @@ -203,9 +207,8 @@ DWORD IsReparsePoint(const base::string16& full_path, bool* result) { 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. *result = false; return ERROR_SUCCESS; @@ -226,11 +229,11 @@ bool SameObject(HANDLE handle, const wchar_t* 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; @@ -239,40 +242,44 @@ bool SameObject(HANDLE handle, const wchar_t* full_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(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; } 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 fe662ae4fde..4a7b1b805c9 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 @@ -5,4 +5,5 @@ https://hg.mozilla.org/mozilla-central/rev/efa7518e43b0 https://hg.mozilla.org/mozilla-central/rev/421446ab2347 https://hg.mozilla.org/mozilla-central/rev/beb74056ac2d https://hg.mozilla.org/mozilla-central/rev/7df8d6639971 -https://bugzilla.mozilla.org/show_bug.cgi?id=1157864 sandbox-cdefs.patch +https://hg.mozilla.org/mozilla-central/rev/1178c11561bc +https://bugzilla.mozilla.org/show_bug.cgi?id=1173371 bug1173371part2.patch