Source code

Revision control

Copy as Markdown

Other Tools

# HG changeset patch
# User Bob Owen <bobowencode@gmail.com>
# Date 1631294898 -3600
# Fri Sep 10 18:28:18 2021 +0100
# Node ID adbc9b3051ab7f3c9360f65fe0fc26bd9d9dd499
# Parent 004b5bea4e78db7ecd665173ce4cf6aa0a1af199
Bug 1695556 p1: Allow reparse points in chromium sandbox code.
diff --git a/sandbox/win/src/filesystem_dispatcher.cc b/sandbox/win/src/filesystem_dispatcher.cc
--- a/sandbox/win/src/filesystem_dispatcher.cc
+++ b/sandbox/win/src/filesystem_dispatcher.cc
@@ -92,17 +92,16 @@ bool FilesystemDispatcher::NtCreateFile(
uint32_t create_options) {
if ((create_options & FILE_VALID_OPTION_FLAGS) != create_options) {
// Do not support brokering calls with special information in
// NtCreateFile()'s ea_buffer (FILE_CONTAINS_EXTENDED_CREATE_INFORMATION).
ipc->return_info.nt_status = STATUS_ACCESS_DENIED;
return false;
}
if (!PreProcessName(name)) {
- // The path requested might contain a reparse point.
ipc->return_info.nt_status = STATUS_ACCESS_DENIED;
return true;
}
EvalResult result = EvalPolicy(IpcTag::NTCREATEFILE, *name, desired_access,
create_disposition == FILE_OPEN);
HANDLE handle;
ULONG_PTR io_information = 0;
@@ -123,17 +122,16 @@ bool FilesystemDispatcher::NtCreateFile(
bool FilesystemDispatcher::NtOpenFile(IPCInfo* ipc,
std::wstring* name,
uint32_t attributes,
uint32_t desired_access,
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;
}
EvalResult result =
EvalPolicy(IpcTag::NTOPENFILE, *name, desired_access, true);
HANDLE handle;
ULONG_PTR io_information = 0;
@@ -154,17 +152,16 @@ bool FilesystemDispatcher::NtOpenFile(IP
bool FilesystemDispatcher::NtQueryAttributesFile(IPCInfo* ipc,
std::wstring* name,
uint32_t attributes,
CountedBuffer* info) {
if (sizeof(FILE_BASIC_INFORMATION) != info->Size())
return false;
if (!PreProcessName(name)) {
- // The path requested might contain a reparse point.
ipc->return_info.nt_status = STATUS_ACCESS_DENIED;
return true;
}
EvalResult result = EvalPolicy(IpcTag::NTQUERYATTRIBUTESFILE, *name);
FILE_BASIC_INFORMATION* information =
reinterpret_cast<FILE_BASIC_INFORMATION*>(info->Buffer());
@@ -184,17 +181,16 @@ bool FilesystemDispatcher::NtQueryAttrib
bool FilesystemDispatcher::NtQueryFullAttributesFile(IPCInfo* ipc,
std::wstring* name,
uint32_t attributes,
CountedBuffer* info) {
if (sizeof(FILE_NETWORK_OPEN_INFORMATION) != info->Size())
return false;
if (!PreProcessName(name)) {
- // The path requested might contain a reparse point.
ipc->return_info.nt_status = STATUS_ACCESS_DENIED;
return true;
}
EvalResult result = EvalPolicy(IpcTag::NTQUERYFULLATTRIBUTESFILE, *name);
FILE_NETWORK_OPEN_INFORMATION* information =
reinterpret_cast<FILE_NETWORK_OPEN_INFORMATION*>(info->Buffer());
static_cast<FILE_INFORMATION_CLASS>(info_class);
*nt_status = GetNtExports()->SetInformationFile(
local_handle, io_block, file_info, length, file_info_class);
return true;
}
bool PreProcessName(std::wstring* path) {
@@ -227,17 +223,16 @@ bool FilesystemDispatcher::NtSetInformat
if (!IsSupportedRenameCall(rename_info, length, info_class))
return false;
std::wstring name;
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;
}
EvalResult result = EvalPolicy(IpcTag::NTSETINFO_RENAME, name);
IO_STATUS_BLOCK* io_status =
reinterpret_cast<IO_STATUS_BLOCK*>(status->Buffer());
diff --git a/sandbox/win/src/filesystem_policy.cc b/sandbox/win/src/filesystem_policy.cc
--- a/sandbox/win/src/filesystem_policy.cc
+++ b/sandbox/win/src/filesystem_policy.cc
@@ -5,16 +5,17 @@
#include "sandbox/win/src/filesystem_policy.h"
#include <windows.h>
#include <winternl.h>
#include <ntstatus.h>
#include <stdint.h>
+#include <algorithm>
#include <string>
#include "base/notreached.h"
#include "base/win/scoped_handle.h"
#include "sandbox/win/src/internal_types.h"
#include "sandbox/win/src/ipc_tags.h"
#include "sandbox/win/src/nt_internals.h"
#include "sandbox/win/src/policy_engine_opcodes.h"
@@ -59,22 +60,16 @@ NTSTATUS NtCreateFileInTarget(HANDLE* ta
NTSTATUS status = GetNtExports()->CreateFile(
&local_handle, desired_access, obj_attributes, io_status_block, nullptr,
file_attributes, share_access, create_disposition, create_options,
ea_buffer, ea_length);
if (!NT_SUCCESS(status)) {
return status;
}
- if (!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)) {
return STATUS_ACCESS_DENIED;
}
return STATUS_SUCCESS;
}
@@ -285,23 +280,32 @@ bool FileSystemPolicy::SetInformationFil
static_cast<FILE_INFORMATION_CLASS>(info_class);
*nt_status = GetNtExports()->SetInformationFile(
local_handle, io_block, file_info, length, file_info_class);
return true;
}
bool PreProcessName(std::wstring* path) {
- ConvertToLongPath(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,
+ // but this is also used for checking policy rules, so we just replace forward
+ // slashes with backslashes.
+ std::replace(path->begin(), path->end(), L'/', L'\\');
+ if (path->find(L"\\..\\") != std::wstring::npos) {
+ return false;
+ }
- if (ERROR_NOT_A_REPARSE_POINT == IsReparsePoint(*path))
- return true;
-
- // We can't process a reparsed file.
- return false;
+ ConvertToLongPath(path);
+ return true;
}
std::wstring FixNTPrefixForMatch(const std::wstring& name) {
std::wstring mod_name = name;
// NT prefix escaped for rule matcher
const wchar_t kNTPrefixEscaped[] = L"\\/?/?\\";
const int kNTPrefixEscapedLen = base::size(kNTPrefixEscaped) - 1;