Skip to content

Commit e5680de

Browse files
committed
[Backtracing] Security improvements.
Use `task_read_for_pid()` rather than having the crashing program pass its own task port through. This opts us in to additional OS security measures surrounding the use of this call. rdar://107362003
1 parent ef46563 commit e5680de

File tree

6 files changed

+54
-49
lines changed

6 files changed

+54
-49
lines changed

stdlib/public/SwiftShims/swift/shims/_SwiftBacktracing.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,8 @@ _swift_backtrace_thread_get_state(__swift_thread_t target_act,
252252
return thread_get_state(target_act, flavor, old_state, old_stateCnt);
253253
}
254254

255+
extern __swift_kern_return_t task_read_for_pid(__swift_task_t task, int pid, __swift_task_t *ptask);
256+
255257
/* DANGER! These are SPI. They may change (or vanish) at short notice, may
256258
not work how you expect, and are generally dangerous to use. */
257259
struct dyld_process_cache_info {
@@ -270,6 +272,11 @@ extern void _dyld_process_info_get_cache(dyld_process_info info, dyld_process_c
270272
extern void _dyld_process_info_for_each_image(dyld_process_info info, void (^callback)(__swift_uint64_t machHeaderAddress, const __swift_uuid_t uuid, const char* path));
271273
extern void _dyld_process_info_for_each_segment(dyld_process_info info, __swift_uint64_t machHeaderAddress, void (^callback)(__swift_uint64_t segmentAddress, __swift_uint64_t segmentSize, const char* segmentName));
272274

275+
#define CS_OPS_STATUS 0
276+
#define CS_PLATFORM_BINARY 0x04000000
277+
#define CS_PLATFORM_PATH 0x08000000
278+
extern int csops(int, unsigned int, void *, __swift_size_t);
279+
273280
/* DANGER! CoreSymbolication is a private framework. This is all SPI. */
274281
typedef __swift_int32_t cpu_type_t;
275282
typedef __swift_int32_t cpu_subtype_t;

stdlib/public/libexec/swift-backtrace/Target.swift

Lines changed: 33 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -101,31 +101,13 @@ class Target {
101101

102102
var mcontext: MContext
103103

104-
static func getParentTask() -> task_t? {
105-
var ports: mach_port_array_t? = nil
106-
var portCount: mach_msg_type_number_t = 0
107-
108-
// For some reason, we can't pass a task read port this way, but we
109-
// *can* pass the control port. So do that and then ask for a read port
110-
// before immediately dropping the control port from this process.
111-
112-
let kr = mach_ports_lookup(mach_task_self_, &ports, &portCount)
104+
static func getTask(pid: pid_t) -> task_t? {
105+
var port: task_t = 0
106+
let kr = task_read_for_pid(mach_task_self_, pid, &port)
113107
if kr != KERN_SUCCESS {
114108
return nil
115109
}
116-
117-
if let ports = ports, portCount != 0 {
118-
var taskPort: mach_port_t = 0
119-
let kr = task_get_special_port(ports[0], TASK_READ_PORT, &taskPort)
120-
if kr != KERN_SUCCESS {
121-
mach_port_deallocate(mach_task_self_, ports[0])
122-
return nil
123-
}
124-
mach_port_deallocate(mach_task_self_, ports[0])
125-
return task_t(taskPort)
126-
} else {
127-
return nil
128-
}
110+
return port
129111
}
130112

131113
static func getProcessName(pid: pid_t) -> String {
@@ -141,15 +123,40 @@ class Target {
141123
}
142124
}
143125

126+
static func isPlatformBinary(pid: pid_t) -> Bool {
127+
var flags = UInt32(0)
128+
129+
return csops(pid,
130+
UInt32(CS_OPS_STATUS),
131+
&flags,
132+
MemoryLayout<UInt32>.size) != 0 ||
133+
(flags & UInt32(CS_PLATFORM_BINARY | CS_PLATFORM_PATH)) != 0
134+
}
135+
144136
init(crashInfoAddr: UInt64, limit: Int?, top: Int, cache: Bool) {
145137
pid = getppid()
146-
if let parentTask = Self.getParentTask() {
147-
task = parentTask
148-
} else {
149-
print("swift-backtrace: couldn't fetch parent task", to: &standardError)
138+
139+
if Self.isPlatformBinary(pid: pid) {
140+
/* Exit silently in this case; either
141+
142+
1. We can't call csops(), because we're sandboxed, or
143+
2. The target is a platform binary.
144+
145+
If we get killed, that is also fine. */
150146
exit(1)
151147
}
152148

149+
// This will normally only succeed if the parent process has
150+
// the com.apple.security.get-task-allow privilege. That gets set
151+
// automatically if you're developing in Xcode; if you're developing
152+
// on the command line or using SwiftPM, you will need to code sign
153+
// your binary with that entitlement to get this to work.
154+
guard let parentTask = Self.getTask(pid: pid) else {
155+
exit(1)
156+
}
157+
158+
task = parentTask
159+
153160
reader = RemoteMemoryReader(task: __swift_task_t(task))
154161

155162
name = Self.getProcessName(pid: pid)

stdlib/public/runtime/CrashHandlerMacOS.cpp

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -342,26 +342,6 @@ trueOrFalse(OnOffTty oot) {
342342
bool
343343
run_backtracer()
344344
{
345-
// Forward our task port to the backtracer; we use the same technique that
346-
// libxpc uses to forward one of its ports on fork(), except that we aren't
347-
// going to call fork() so libxpc's atfork handler won't run and we'll get
348-
// to send the task port to the child.
349-
//
350-
// I would very much like to send a task *read* port, but for some reason
351-
// that doesn't work here. As a result, what we do instead is send the
352-
// control port but have the backtracer use it to get the read port and
353-
// immediately drop the control port.
354-
//
355-
// That *should* be safe enough in practice; if someone could replace the
356-
// backtracer, then they can also replace libswiftCore, and since we do
357-
// this early on in backtracer start-up, the control port won't be valid
358-
// by the time anyone gets to try anything nefarious.
359-
mach_port_t ports[] = {
360-
mach_task_self(),
361-
};
362-
363-
mach_ports_register(mach_task_self(), ports, 1);
364-
365345
// Set-up the backtracer's command line arguments
366346
switch (_swift_backtraceSettings.algorithm) {
367347
case UnwindAlgorithm::Fast:

test/lit.cfg

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1024,7 +1024,8 @@ if run_vendor == 'apple':
10241024
if 'use_os_stdlib' not in lit_config.params:
10251025
config.target_codesign = make_path(config.swift_utils, "swift-darwin-postprocess.py")
10261026
else:
1027-
config.target_codesign = "codesign -f -s -"
1027+
config.target_codesign = "codesign -f -s - --options=runtime --entitlements {}".format(os.path.join(config.swift_utils, 'get-task-allow.plist'))
1028+
10281029
config.target_library_path_var = "DYLD_LIBRARY_PATH"
10291030
config.target_runtime = "objc"
10301031
config.target_sdk_libcxx_path = os.path.join(config.variant_sdk, 'usr', 'include', 'c++', 'v1')

utils/get-task-allow.plist

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
3+
<plist version="1.0">
4+
<dict>
5+
<key>com.apple.security.get-task-allow</key>
6+
<true/>
7+
</dict>
8+
</plist>

utils/swift-darwin-postprocess.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
import sys
1111

1212
utils = os.path.dirname(os.path.realpath(__file__))
13-
13+
get_task_allow_plist = os.path.join(utils, 'get-task-allow.plist')
1414

1515
def main(arguments):
1616
parser = argparse.ArgumentParser(
@@ -91,7 +91,9 @@ def unrpathize(filename):
9191

9292
def codesign(filename):
9393
# "-" is the signing identity for ad-hoc signing.
94-
command = ["/usr/bin/codesign", "--force", "--sign", "-", filename]
94+
command = ['/usr/bin/codesign', '--force', '--sign', '-',
95+
'--options=runtime', '--entitlements', get_task_allow_plist,
96+
filename]
9597
subprocess.check_call(command)
9698

9799

0 commit comments

Comments
 (0)