From 6a2afca228ba18a0ffd20657c5f72e9115346ae8 Mon Sep 17 00:00:00 2001
From: Chris Bracken <chris@bracken.jp>
Date: Wed, 2 Aug 2023 14:24:53 -0700
Subject: [PATCH] [iOS] Fix use-after-free in setBinaryMessenger (#44294)

Previously, when setting the binary messenger to the current binary messenger, we were freeing the current binary messenger before setting the new (current) binary messenger, triggering a use after free.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
---
 .../ios/framework/Source/FlutterEngine.mm     |  8 +++--
 .../ios/framework/Source/FlutterEngineTest.mm | 30 +++++++++++++++++++
 2 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm
index 37451fd2da1bf..00637023f3e6e 100644
--- a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm
+++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm
@@ -1217,9 +1217,11 @@ - (void)flutterViewAccessibilityDidCall {
 // remove this.
 - (void)setBinaryMessenger:(FlutterBinaryMessengerRelay*)binaryMessenger {
   // Discard the previous messenger and keep the new one.
-  _binaryMessenger.parent = nil;
-  [_binaryMessenger release];
-  _binaryMessenger = [binaryMessenger retain];
+  if (binaryMessenger != _binaryMessenger) {
+    _binaryMessenger.parent = nil;
+    [_binaryMessenger release];
+    _binaryMessenger = [binaryMessenger retain];
+  }
 }
 
 #pragma mark - FlutterBinaryMessenger
diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm
index 34743904f7cf6..64167b7496c45 100644
--- a/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm
+++ b/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm
@@ -22,6 +22,22 @@ @interface FlutterEngine () <FlutterTextInputDelegate>
 
 @end
 
+/// FlutterBinaryMessengerRelay used for testing that setting FlutterEngine.binaryMessenger to
+/// the current instance doesn't trigger a use-after-free bug.
+///
+/// See: testSetBinaryMessengerToSameBinaryMessenger
+@interface FakeBinaryMessengerRelay : FlutterBinaryMessengerRelay
+@property(nonatomic, assign) BOOL failOnDealloc;
+@end
+
+@implementation FakeBinaryMessengerRelay
+- (void)dealloc {
+  if (_failOnDealloc) {
+    XCTFail("FakeBinaryMessageRelay should not be deallocated");
+  }
+}
+@end
+
 @interface FlutterEngineTest : XCTestCase
 @end
 
@@ -148,6 +164,20 @@ - (void)testNotifyPluginOfDealloc {
   OCMVerify([plugin detachFromEngineForRegistrar:[OCMArg any]]);
 }
 
+- (void)testSetBinaryMessengerToSameBinaryMessenger {
+  FakeBinaryMessengerRelay* fakeBinaryMessenger = [[FakeBinaryMessengerRelay alloc] init];
+
+  FlutterEngine* engine = [[FlutterEngine alloc] init];
+  [engine setBinaryMessenger:fakeBinaryMessenger];
+
+  // Verify that the setter doesn't free the old messenger before setting the new messenger.
+  fakeBinaryMessenger.failOnDealloc = YES;
+  [engine setBinaryMessenger:fakeBinaryMessenger];
+
+  // Don't fail when ARC releases the binary messenger.
+  fakeBinaryMessenger.failOnDealloc = NO;
+}
+
 - (void)testRunningInitialRouteSendsNavigationMessage {
   id mockBinaryMessenger = OCMClassMock([FlutterBinaryMessengerRelay class]);