From 2b11c801455754d574467c374dc1086ffe8f8964 Mon Sep 17 00:00:00 2001
From: Nick Dowell <nick@bugsnag.com>
Date: Wed, 7 Jul 2021 09:57:07 +0100
Subject: [PATCH] Fix stack overflow in +[BugsnagThread
 allThreadsWithCurrentThreadBacktrace:]

---
 Bugsnag/Payload/BugsnagThread.m | 22 +++++++++++++---------
 CHANGELOG.md                    |  3 +++
 Tests/BugsnagThreadTests.m      | 20 ++++++++++++++++++++
 3 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/Bugsnag/Payload/BugsnagThread.m b/Bugsnag/Payload/BugsnagThread.m
index e43384317..a625d93b4 100644
--- a/Bugsnag/Payload/BugsnagThread.m
+++ b/Bugsnag/Payload/BugsnagThread.m
@@ -213,19 +213,23 @@ + (NSDictionary *)enhanceThreadInfo:(NSDictionary *)thread
     thread_t *threads = NULL;
     mach_msg_type_number_t threadCount = 0;
 
+    if (task_threads(mach_task_self(), &threads, &threadCount) != KERN_SUCCESS) {
+        return @[];
+    }
+
+    NSMutableArray *objects = [NSMutableArray arrayWithCapacity:threadCount];
+
+    struct backtrace_t *backtraces = calloc(threadCount, sizeof(struct backtrace_t));
+    if (!backtraces) {
+        goto cleanup;
+    }
+
     suspend_threads();
 
     // While threads are suspended only async-signal-safe functions should be used,
     // as another threads may have been suspended while holding a lock in malloc,
     // the Objective-C runtime, or other subsystems.
 
-    if (task_threads(mach_task_self(), &threads, &threadCount) != KERN_SUCCESS) {
-        resume_threads();
-        return @[];
-    }
-
-    struct backtrace_t backtraces[threadCount];
-
     for (mach_msg_type_number_t i = 0; i < threadCount; i++) {
         BOOL isCurrentThread = MACH_PORT_INDEX(threads[i]) == MACH_PORT_INDEX(bsg_ksmachthread_self());
         if (isCurrentThread) {
@@ -237,8 +241,6 @@ + (NSDictionary *)enhanceThreadInfo:(NSDictionary *)thread
 
     resume_threads();
 
-    NSMutableArray *objects = [NSMutableArray arrayWithCapacity:threadCount];
-
     for (mach_msg_type_number_t i = 0; i < threadCount; i++) {
         BOOL isCurrentThread = MACH_PORT_INDEX(threads[i]) == MACH_PORT_INDEX(bsg_ksmachthread_self());
         struct backtrace_t *backtrace = isCurrentThread ? currentThreadBacktrace : &backtraces[i];
@@ -249,11 +251,13 @@ + (NSDictionary *)enhanceThreadInfo:(NSDictionary *)thread
                                                                index:i]];
     }
 
+cleanup:
     for (mach_msg_type_number_t i = 0; i < threadCount; i++) {
         mach_port_deallocate(mach_task_self(), threads[i]);
     }
     vm_deallocate(mach_task_self(), (vm_address_t)threads, sizeof(thread_t) * threadCount);
 
+    free(backtraces);
     return objects;
 }
 
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d96391692..cc9706194 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,9 @@ Changelog
 
 ### Bug fixes
 
+* Fix a potential stack overflow in `+[BugsnagThread allThreadsWithCurrentThreadBacktrace:]`.
+  [#1148](https://github.com/bugsnag/bugsnag-cocoa/pull/1148)
+
 * Fix `NSNull` handling in `+[BugsnagError errorFromJson:]` and `+[BugsnagStackframe frameFromJson:]`.
   [#1143](https://github.com/bugsnag/bugsnag-cocoa/pull/1143)
   [#1138](https://github.com/bugsnag/bugsnag-cocoa/issues/1138)
diff --git a/Tests/BugsnagThreadTests.m b/Tests/BugsnagThreadTests.m
index b6e1f629a..c27fd443e 100644
--- a/Tests/BugsnagThreadTests.m
+++ b/Tests/BugsnagThreadTests.m
@@ -222,6 +222,26 @@ - (void)testAllThreads {
     XCTAssertGreaterThan(threads.count, 1);
 }
 
+- (void)testAllThreadsFromBackgroundDoesNotOverflowStack {
+    const int threadCount = 1000;
+    dispatch_semaphore_t semaphore = dispatch_semaphore_create(0);
+    for (int i = 0; i < threadCount; i++) {
+        [NSThread detachNewThreadSelector:@selector(waitForSemaphore:) toTarget:self withObject:semaphore];
+    }
+    __block NSArray<BugsnagThread *> *threads = nil;
+    dispatch_sync(dispatch_get_global_queue(QOS_CLASS_USER_INITIATED, 0), ^{
+        threads = [BugsnagThread allThreads:YES callStackReturnAddresses:NSThread.callStackReturnAddresses];
+    });
+    XCTAssertGreaterThan(threads.count, threadCount);
+    for (int i = 0; i < threadCount; i++) {
+        dispatch_semaphore_signal(semaphore);
+    }
+}
+
+- (void)waitForSemaphore:(dispatch_semaphore_t)semaphore {
+    dispatch_semaphore_wait(semaphore, DISPATCH_TIME_FOREVER);
+}
+
 - (void)testCurrentThread {
     NSArray<BugsnagThread *> *threads = [BugsnagThread allThreads:NO callStackReturnAddresses:NSThread.callStackReturnAddresses];
     [threads[0].stacktrace makeObjectsPerformSelector:@selector(symbolicateIfNeeded)];