diff --git a/security/sandbox/chromium/sandbox/win/src/filesystem_dispatcher.cc b/security/sandbox/chromium/sandbox/win/src/filesystem_dispatcher.cc index ff0f8885cd64..3452c42907fe 100644 --- a/security/sandbox/chromium/sandbox/win/src/filesystem_dispatcher.cc +++ b/security/sandbox/chromium/sandbox/win/src/filesystem_dispatcher.cc @@ -92,7 +92,6 @@ bool FilesystemDispatcher::NtCreateFile(IPCInfo* ipc, uint32_t create_disposition, uint32_t create_options) { if (!PreProcessName(name)) { - // The path requested might contain a reparse point. ipc->return_info.nt_status = STATUS_ACCESS_DENIED; return true; } @@ -146,7 +145,6 @@ bool FilesystemDispatcher::NtOpenFile(IPCInfo* ipc, uint32_t share_access, uint32_t open_options) { if (!PreProcessName(name)) { - // The path requested might contain a reparse point. ipc->return_info.nt_status = STATUS_ACCESS_DENIED; return true; } @@ -201,7 +199,6 @@ bool FilesystemDispatcher::NtQueryAttributesFile(IPCInfo* ipc, return false; if (!PreProcessName(name)) { - // The path requested might contain a reparse point. ipc->return_info.nt_status = STATUS_ACCESS_DENIED; return true; } @@ -250,7 +247,6 @@ bool FilesystemDispatcher::NtQueryFullAttributesFile(IPCInfo* ipc, return false; if (!PreProcessName(name)) { - // The path requested might contain a reparse point. ipc->return_info.nt_status = STATUS_ACCESS_DENIED; return true; } @@ -312,7 +308,6 @@ bool FilesystemDispatcher::NtSetInformationFile(IPCInfo* ipc, name.assign(rename_info->FileName, rename_info->FileNameLength / sizeof(rename_info->FileName[0])); if (!PreProcessName(&name)) { - // The path requested might contain a reparse point. ipc->return_info.nt_status = STATUS_ACCESS_DENIED; return true; } diff --git a/security/sandbox/chromium/sandbox/win/src/filesystem_policy.cc b/security/sandbox/chromium/sandbox/win/src/filesystem_policy.cc index da9ef58a4f5b..9d1149eda549 100644 --- a/security/sandbox/chromium/sandbox/win/src/filesystem_policy.cc +++ b/security/sandbox/chromium/sandbox/win/src/filesystem_policy.cc @@ -44,12 +44,6 @@ NTSTATUS NtCreateFileInTarget(HANDLE* target_file_handle, return status; } - if (!sandbox::SameObject(local_handle, obj_attributes->ObjectName->Buffer)) { - // The handle points somewhere else. Fail the operation. - ::CloseHandle(local_handle); - return STATUS_ACCESS_DENIED; - } - if (!::DuplicateHandle(::GetCurrentProcess(), local_handle, target_process, target_file_handle, 0, false, DUPLICATE_CLOSE_SOURCE | DUPLICATE_SAME_ACCESS)) { @@ -405,13 +399,25 @@ bool FileSystemPolicy::SetInformationFileAction(EvalResult eval_result, } bool PreProcessName(std::wstring* path) { + // We now allow symbolic links to be opened via the broker, so we can no + // longer rely on the same object check where we checked the path of the + // opened file against the original. We don't specify a root when creating + // OBJECT_ATTRIBUTES from file names for brokering so they must be fully + // qualified and we can just check for the parent directory double dot between + // two backslashes. NtCreateFile doesn't seem to allow it anyway, but this is + // just an extra precaution. It also doesn't seem to allow the forward slash + // at least in fully qualified names so we rule out that as well, to simplify + // the combinations we might have to check. + if (path->find(L'/') != std::wstring::npos) { + return false; + } + + if (path->find(L"\\..\\") != std::wstring::npos) { + return false; + } + ConvertToLongPath(path); - - if (ERROR_NOT_A_REPARSE_POINT == IsReparsePoint(*path)) - return true; - - // We can't process a reparsed file. - return false; + return true; } std::wstring FixNTPrefixForMatch(const std::wstring& name) {