diff options
author | forshaw <forshaw@chromium.org> | 2014-12-18 03:54:31 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2014-12-18 11:54:49 +0000 |
commit | 14976c19a6ab3cd23458d0a1616e7c5fe970c64d (patch) | |
tree | 81bf805189ebb71a258a9ca66268a46717e765c3 | |
parent | 826eced0a2aa1c102999577e3730ed08c623226d (diff) | |
download | chromium_src-14976c19a6ab3cd23458d0a1616e7c5fe970c64d.zip chromium_src-14976c19a6ab3cd23458d0a1616e7c5fe970c64d.tar.gz chromium_src-14976c19a6ab3cd23458d0a1616e7c5fe970c64d.tar.bz2 |
Changes to the handling of impersonation levels for Windows pipes.
This patch makes some changes to impersonation levels for pipes used
in IPC. It changes the default level for the IPC channel client to
use SECURITY_ANONYMOUS and also modified the sandbox's file policy
code to always pass a SecurityAnonymous QOS structure to all file
creations and opens. This was done to avoid modifying the IPC calls
themselves to add the QOS information. The aim of the patch is to
reduce the possibility of impersonation tokens leaking to less trusted
processes during normal operation.
BUG=440694
Review URL: https://codereview.chromium.org/799943002
Cr-Commit-Position: refs/heads/master@{#308993}
-rw-r--r-- | sandbox/win/src/filesystem_policy.cc | 32 | ||||
-rw-r--r-- | sandbox/win/src/registry_policy.cc | 4 | ||||
-rw-r--r-- | sandbox/win/src/sandbox_utils.cc | 4 | ||||
-rw-r--r-- | sandbox/win/src/sandbox_utils.h | 3 | ||||
-rw-r--r-- | sandbox/win/src/sync_policy.cc | 10 | ||||
-rw-r--r-- | sandbox/win/src/win_utils.cc | 14 | ||||
-rw-r--r-- | sandbox/win/src/win_utils.h | 3 | ||||
-rw-r--r-- | sandbox/win/src/win_utils_unittest.cc | 28 |
8 files changed, 79 insertions, 19 deletions
diff --git a/sandbox/win/src/filesystem_policy.cc b/sandbox/win/src/filesystem_policy.cc index 87340a8..65e8f47 100644 --- a/sandbox/win/src/filesystem_policy.cc +++ b/sandbox/win/src/filesystem_policy.cc @@ -54,6 +54,18 @@ NTSTATUS NtCreateFileInTarget(HANDLE* target_file_handle, return STATUS_SUCCESS; } +// Get an initialized anonymous level Security QOS. +SECURITY_QUALITY_OF_SERVICE GetAnonymousQOS() { + SECURITY_QUALITY_OF_SERVICE security_qos = {0}; + security_qos.Length = sizeof(security_qos); + security_qos.ImpersonationLevel = SecurityAnonymous; + // Set dynamic tracking so that a pipe doesn't capture the broker's token + security_qos.ContextTrackingMode = SECURITY_DYNAMIC_TRACKING; + security_qos.EffectiveOnly = TRUE; + + return security_qos; +} + } // namespace. namespace sandbox { @@ -245,7 +257,10 @@ bool FileSystemPolicy::CreateFileAction(EvalResult eval_result, IO_STATUS_BLOCK io_block = {0}; UNICODE_STRING uni_name = {0}; OBJECT_ATTRIBUTES obj_attributes = {0}; - InitObjectAttribs(file, attributes, NULL, &obj_attributes, &uni_name); + SECURITY_QUALITY_OF_SERVICE security_qos = GetAnonymousQOS(); + + InitObjectAttribs(file, attributes, NULL, &obj_attributes, + &uni_name, IsPipe(file) ? &security_qos : NULL); *nt_status = NtCreateFileInTarget(handle, desired_access, &obj_attributes, &io_block, file_attributes, share_access, create_disposition, create_options, NULL, @@ -276,7 +291,10 @@ bool FileSystemPolicy::OpenFileAction(EvalResult eval_result, IO_STATUS_BLOCK io_block = {0}; UNICODE_STRING uni_name = {0}; OBJECT_ATTRIBUTES obj_attributes = {0}; - InitObjectAttribs(file, attributes, NULL, &obj_attributes, &uni_name); + SECURITY_QUALITY_OF_SERVICE security_qos = GetAnonymousQOS(); + + InitObjectAttribs(file, attributes, NULL, &obj_attributes, + &uni_name, IsPipe(file) ? &security_qos : NULL); *nt_status = NtCreateFileInTarget(handle, desired_access, &obj_attributes, &io_block, 0, share_access, FILE_OPEN, open_options, NULL, 0, @@ -305,7 +323,10 @@ bool FileSystemPolicy::QueryAttributesFileAction( UNICODE_STRING uni_name = {0}; OBJECT_ATTRIBUTES obj_attributes = {0}; - InitObjectAttribs(file, attributes, NULL, &obj_attributes, &uni_name); + SECURITY_QUALITY_OF_SERVICE security_qos = GetAnonymousQOS(); + + InitObjectAttribs(file, attributes, NULL, &obj_attributes, + &uni_name, IsPipe(file) ? &security_qos : NULL); *nt_status = NtQueryAttributesFile(&obj_attributes, file_info); return true; @@ -330,7 +351,10 @@ bool FileSystemPolicy::QueryFullAttributesFileAction( UNICODE_STRING uni_name = {0}; OBJECT_ATTRIBUTES obj_attributes = {0}; - InitObjectAttribs(file, attributes, NULL, &obj_attributes, &uni_name); + SECURITY_QUALITY_OF_SERVICE security_qos = GetAnonymousQOS(); + + InitObjectAttribs(file, attributes, NULL, &obj_attributes, + &uni_name, IsPipe(file) ? &security_qos : NULL); *nt_status = NtQueryFullAttributesFile(&obj_attributes, file_info); return true; diff --git a/sandbox/win/src/registry_policy.cc b/sandbox/win/src/registry_policy.cc index 4c75d23..b0f24a7 100644 --- a/sandbox/win/src/registry_policy.cc +++ b/sandbox/win/src/registry_policy.cc @@ -191,7 +191,7 @@ bool RegistryPolicy::CreateKeyAction(EvalResult eval_result, UNICODE_STRING uni_name = {0}; OBJECT_ATTRIBUTES obj_attributes = {0}; InitObjectAttribs(key, attributes, root_directory, &obj_attributes, - &uni_name); + &uni_name, NULL); *nt_status = NtCreateKeyInTarget(handle, desired_access, &obj_attributes, title_index, NULL, create_options, disposition, client_info.process); @@ -216,7 +216,7 @@ bool RegistryPolicy::OpenKeyAction(EvalResult eval_result, UNICODE_STRING uni_name = {0}; OBJECT_ATTRIBUTES obj_attributes = {0}; InitObjectAttribs(key, attributes, root_directory, &obj_attributes, - &uni_name); + &uni_name, NULL); *nt_status = NtOpenKeyInTarget(handle, desired_access, &obj_attributes, client_info.process); return true; diff --git a/sandbox/win/src/sandbox_utils.cc b/sandbox/win/src/sandbox_utils.cc index a1b77d6..6057caf 100644 --- a/sandbox/win/src/sandbox_utils.cc +++ b/sandbox/win/src/sandbox_utils.cc @@ -15,7 +15,8 @@ void InitObjectAttribs(const base::string16& name, ULONG attributes, HANDLE root, OBJECT_ATTRIBUTES* obj_attr, - UNICODE_STRING* uni_name) { + UNICODE_STRING* uni_name, + SECURITY_QUALITY_OF_SERVICE* security_qos) { static RtlInitUnicodeStringFunction RtlInitUnicodeString; if (!RtlInitUnicodeString) { HMODULE ntdll = ::GetModuleHandle(kNtdllName); @@ -25,6 +26,7 @@ void InitObjectAttribs(const base::string16& name, } RtlInitUnicodeString(uni_name, name.c_str()); InitializeObjectAttributes(obj_attr, uni_name, attributes, root, NULL); + obj_attr->SecurityQualityOfService = security_qos; } } // namespace sandbox diff --git a/sandbox/win/src/sandbox_utils.h b/sandbox/win/src/sandbox_utils.h index 2a17d63..fc3100d 100644 --- a/sandbox/win/src/sandbox_utils.h +++ b/sandbox/win/src/sandbox_utils.h @@ -18,7 +18,8 @@ void InitObjectAttribs(const base::string16& name, ULONG attributes, HANDLE root, OBJECT_ATTRIBUTES* obj_attr, - UNICODE_STRING* uni_name); + UNICODE_STRING* uni_name, + SECURITY_QUALITY_OF_SERVICE* security_qos); } // namespace sandbox diff --git a/sandbox/win/src/sync_policy.cc b/sandbox/win/src/sync_policy.cc index 5c7e0fe..607b446 100644 --- a/sandbox/win/src/sync_policy.cc +++ b/sandbox/win/src/sync_policy.cc @@ -41,7 +41,7 @@ NTSTATUS ResolveSymbolicLink(const base::string16& directory_name, UNICODE_STRING symbolic_link_directory_string = {}; InitObjectAttribs(directory_name, OBJ_CASE_INSENSITIVE, NULL, &symbolic_link_directory_attributes, - &symbolic_link_directory_string); + &symbolic_link_directory_string, NULL); HANDLE symbolic_link_directory = NULL; NTSTATUS status = NtOpenDirectoryObject(&symbolic_link_directory, @@ -56,7 +56,7 @@ NTSTATUS ResolveSymbolicLink(const base::string16& directory_name, OBJECT_ATTRIBUTES symbolic_link_attributes = {}; UNICODE_STRING name_string = {}; InitObjectAttribs(name, OBJ_CASE_INSENSITIVE, symbolic_link_directory, - &symbolic_link_attributes, &name_string); + &symbolic_link_attributes, &name_string, NULL); HANDLE symbolic_link = NULL; status = NtOpenSymbolicLinkObject(&symbolic_link, GENERIC_READ, @@ -121,7 +121,7 @@ NTSTATUS GetBaseNamedObjectsDirectory(HANDLE* directory) { UNICODE_STRING directory_name = {}; OBJECT_ATTRIBUTES object_attributes = {}; InitObjectAttribs(base_named_objects_path, OBJ_CASE_INSENSITIVE, NULL, - &object_attributes, &directory_name); + &object_attributes, &directory_name, NULL); status = NtOpenDirectoryObject(&base_named_objects_handle, DIRECTORY_ALL_ACCESS, &object_attributes); @@ -198,7 +198,7 @@ NTSTATUS SyncPolicy::CreateEventAction(EvalResult eval_result, UNICODE_STRING unicode_event_name = {}; OBJECT_ATTRIBUTES object_attributes = {}; InitObjectAttribs(event_name, OBJ_CASE_INSENSITIVE, object_directory, - &object_attributes, &unicode_event_name); + &object_attributes, &unicode_event_name, NULL); HANDLE local_handle = NULL; status = NtCreateEvent(&local_handle, EVENT_ALL_ACCESS, &object_attributes, @@ -236,7 +236,7 @@ NTSTATUS SyncPolicy::OpenEventAction(EvalResult eval_result, UNICODE_STRING unicode_event_name = {}; OBJECT_ATTRIBUTES object_attributes = {}; InitObjectAttribs(event_name, OBJ_CASE_INSENSITIVE, object_directory, - &object_attributes, &unicode_event_name); + &object_attributes, &unicode_event_name, NULL); HANDLE local_handle = NULL; status = NtOpenEvent(&local_handle, desired_access, &object_attributes); diff --git a/sandbox/win/src/win_utils.cc b/sandbox/win/src/win_utils.cc index ea68c07..12f8189 100644 --- a/sandbox/win/src/win_utils.cc +++ b/sandbox/win/src/win_utils.cc @@ -7,6 +7,7 @@ #include <map> #include "base/memory/scoped_ptr.h" +#include "base/strings/string_util.h" #include "base/win/pe_image.h" #include "sandbox/win/src/internal_types.h" #include "sandbox/win/src/nt_internals.h" @@ -33,20 +34,21 @@ const KnownReservedKey kKnownKey[] = { { L"HKEY_DYN_DATA", HKEY_DYN_DATA} }; +} // namespace + +namespace sandbox { + // Returns true if the provided path points to a pipe. bool IsPipe(const base::string16& path) { + base::string16 lower_path = base::StringToLowerASCII(path); size_t start = 0; - if (0 == path.compare(0, sandbox::kNTPrefixLen, sandbox::kNTPrefix)) + if (0 == lower_path.compare(0, sandbox::kNTPrefixLen, sandbox::kNTPrefix)) start = sandbox::kNTPrefixLen; const wchar_t kPipe[] = L"pipe\\"; - return (0 == path.compare(start, arraysize(kPipe) - 1, kPipe)); + return (0 == lower_path.compare(start, arraysize(kPipe) - 1, kPipe)); } -} // namespace - -namespace sandbox { - HKEY GetReservedKeyFromName(const base::string16& name) { for (size_t i = 0; i < arraysize(kKnownKey); ++i) { if (name == kKnownKey[i].name) diff --git a/sandbox/win/src/win_utils.h b/sandbox/win/src/win_utils.h index 9b58d1d..f576492 100644 --- a/sandbox/win/src/win_utils.h +++ b/sandbox/win/src/win_utils.h @@ -105,6 +105,9 @@ bool ResolveRegistryName(base::string16 name, base::string16* resolved_name); bool WriteProtectedChildMemory(HANDLE child_process, void* address, const void* buffer, size_t length); +// Returns true if the provided path points to a pipe. +bool IsPipe(const base::string16& path); + } // namespace sandbox // Resolves a function name in NTDLL to a function pointer. The second parameter diff --git a/sandbox/win/src/win_utils_unittest.cc b/sandbox/win/src/win_utils_unittest.cc index 3736654..569acff 100644 --- a/sandbox/win/src/win_utils_unittest.cc +++ b/sandbox/win/src/win_utils_unittest.cc @@ -81,3 +81,31 @@ TEST(WinUtils, SameObject) { file.Close(); EXPECT_TRUE(::RemoveDirectory(my_folder)); } + +TEST(WinUtils, IsPipe) { + using sandbox::IsPipe; + + base::string16 pipe_name = L"\\??\\pipe\\mypipe"; + EXPECT_TRUE(IsPipe(pipe_name)); + + pipe_name = L"\\??\\PiPe\\mypipe"; + EXPECT_TRUE(IsPipe(pipe_name)); + + pipe_name = L"\\??\\pipe"; + EXPECT_FALSE(IsPipe(pipe_name)); + + pipe_name = L"\\??\\_pipe_\\mypipe"; + EXPECT_FALSE(IsPipe(pipe_name)); + + pipe_name = L"\\??\\ABCD\\mypipe"; + EXPECT_FALSE(IsPipe(pipe_name)); + + pipe_name = L"/??/pipe/mypipe"; + EXPECT_FALSE(IsPipe(pipe_name)); + + pipe_name = L"\\XX\\pipe\\mypipe"; + EXPECT_FALSE(IsPipe(pipe_name)); + + pipe_name = L"\\Device\\NamedPipe\\mypipe"; + EXPECT_FALSE(IsPipe(pipe_name)); +} |