1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
|
# 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.
Differential Revision: https://phabricator.services.mozilla.com/D135692
diff --git a/security/sandbox/chromium/sandbox/win/src/filesystem_dispatcher.cc b/security/sandbox/chromium/sandbox/win/src/filesystem_dispatcher.cc
--- a/security/sandbox/chromium/sandbox/win/src/filesystem_dispatcher.cc
+++ b/security/sandbox/chromium/sandbox/win/src/filesystem_dispatcher.cc
@@ -87,17 +87,16 @@ bool FilesystemDispatcher::NtCreateFile(
std::wstring* name,
uint32_t attributes,
uint32_t desired_access,
uint32_t file_attributes,
uint32_t share_access,
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;
}
const wchar_t* filename = name->c_str();
uint32_t broker = BROKER_TRUE;
CountedParameterSet<OpenFile> params;
@@ -141,17 +140,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;
}
const wchar_t* filename = name->c_str();
uint32_t broker = BROKER_TRUE;
uint32_t create_disposition = FILE_OPEN;
@@ -196,17 +194,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;
}
uint32_t broker = BROKER_TRUE;
const wchar_t* filename = name->c_str();
CountedParameterSet<FileName> params;
params[FileName::NAME] = ParamPickerMake(filename);
@@ -245,17 +242,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;
}
uint32_t broker = BROKER_TRUE;
const wchar_t* filename = name->c_str();
CountedParameterSet<FileName> params;
params[FileName::NAME] = ParamPickerMake(filename);
@@ -307,17 +303,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;
}
uint32_t broker = BROKER_TRUE;
const wchar_t* filename = name.c_str();
CountedParameterSet<FileName> params;
params[FileName::NAME] = ParamPickerMake(filename);
diff --git a/security/sandbox/chromium/sandbox/win/src/filesystem_policy.cc b/security/sandbox/chromium/sandbox/win/src/filesystem_policy.cc
--- a/security/sandbox/chromium/sandbox/win/src/filesystem_policy.cc
+++ b/security/sandbox/chromium/sandbox/win/src/filesystem_policy.cc
@@ -1,16 +1,17 @@
// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "sandbox/win/src/filesystem_policy.h"
#include <stdint.h>
+#include <algorithm>
#include <string>
#include "base/logging.h"
#include "base/stl_util.h"
#include "base/win/scoped_handle.h"
#include "base/win/windows_version.h"
#include "sandbox/win/src/ipc_tags.h"
#include "sandbox/win/src/policy_engine_opcodes.h"
@@ -39,22 +40,16 @@ NTSTATUS NtCreateFileInTarget(HANDLE* ta
NTSTATUS status =
NtCreateFile(&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 (!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)) {
return STATUS_ACCESS_DENIED;
}
return STATUS_SUCCESS;
}
@@ -400,23 +395,32 @@ bool FileSystemPolicy::SetInformationFil
static_cast<FILE_INFORMATION_CLASS>(info_class);
*nt_status = NtSetInformationFile(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;
|
