[iOSTextInput] fix potential dangling pointer access (flutter/engine#26547)

2026-02-20 02:29:02 +08:00 · 2021-06-03 20:54:01 -07:00 · 2021-06-03 20:54:01 -07:00 · 4e1ca7bbd2
commit 4e1ca7bbd2
parent ca12816fa0
2 changed files with 127 additions and 59 deletions
--- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm
+++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm
@ -505,6 +505,9 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) {
  const char* _selectionAffinity;
  FlutterTextRange* _selectedTextRange;
  CGRect _cachedFirstRect;
+  // The view has reached end of life, and is no longer
+  // allowed to access its textInputDelegate.
+  BOOL _decommissioned;
 }

@synthesize tokenizer = _tokenizer;
@ -535,6 +538,7 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) {
    _returnKeyType = UIReturnKeyDone;
    _secureTextEntry = NO;
    _accessibilityEnabled = NO;
+    _decommissioned = NO;
    if (@available(iOS 11.0, *)) {
      _smartQuotesType = UITextSmartQuotesTypeYes;
      _smartDashesType = UITextSmartDashesTypeYes;
@ -545,6 +549,7 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) {
 }

 - (void)configureWithDictionary:(NSDictionary*)configuration {
+  NSAssert(!_decommissioned, @"Attempt to reuse a decommissioned view, for %@", configuration);
  NSDictionary* inputType = configuration[kKeyboardType];
  NSString* keyboardAppearance = configuration[kKeyboardAppearance];
  NSDictionary* autofill = configuration[kAutofillProperties];
@ -596,6 +601,23 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) {
  return _textContentType;
 }

+- (id<FlutterTextInputDelegate>)textInputDelegate {
+  return _decommissioned ? nil : _textInputDelegate;
+}
+
+// Declares that the view has reached end of life, and
+// is no longer allowed to access its textInputDelegate.
+//
+// UIKit may retain this view (even after it's been removed
+// from the view hierarchy) so that it may outlive the plugin/engine,
+// in which case _textInputDelegate will become a dangling pointer.
+
+// The text input plugin needs to call decommision when it should
+// not have access to its FlutterTextInputDelegate any more.
+- (void)decommision {
+  _decommissioned = YES;
+}
+
 - (void)dealloc {
  [_text release];
  [_markedText release];
@ -778,7 +800,8 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) {

 - (BOOL)shouldChangeTextInRange:(UITextRange*)range replacementText:(NSString*)text {
  if (self.returnKeyType == UIReturnKeyDefault && [text isEqualToString:@"\n"]) {
-    [_textInputDelegate performAction:FlutterTextInputActionNewline withClient:_textInputClient];
+    [self.textInputDelegate performAction:FlutterTextInputActionNewline
+                               withClient:_textInputClient];
    return YES;
  }

@ -819,7 +842,7 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) {
        break;
    }

-    [_textInputDelegate performAction:action withClient:_textInputClient];
+    [self.textInputDelegate performAction:action withClient:_textInputClient];
    return NO;
  }

@ -1062,9 +1085,9 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) {
    return _cachedFirstRect;
  }

-  [_textInputDelegate showAutocorrectionPromptRectForStart:start
-                                                       end:end
-                                                withClient:_textInputClient];
+  [self.textInputDelegate showAutocorrectionPromptRectForStart:start
+                                                           end:end
+                                                    withClient:_textInputClient];
  // TODO(cbracken) Implement.
  return CGRectZero;
 }
@ -1097,21 +1120,21 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) {
 }

 - (void)beginFloatingCursorAtPoint:(CGPoint)point {
-  [_textInputDelegate updateFloatingCursor:FlutterFloatingCursorDragStateStart
-                                withClient:_textInputClient
-                              withPosition:@{@"X" : @(point.x), @"Y" : @(point.y)}];
+  [self.textInputDelegate updateFloatingCursor:FlutterFloatingCursorDragStateStart
+                                    withClient:_textInputClient
+                                  withPosition:@{@"X" : @(point.x), @"Y" : @(point.y)}];
 }

 - (void)updateFloatingCursorAtPoint:(CGPoint)point {
-  [_textInputDelegate updateFloatingCursor:FlutterFloatingCursorDragStateUpdate
-                                withClient:_textInputClient
-                              withPosition:@{@"X" : @(point.x), @"Y" : @(point.y)}];
+  [self.textInputDelegate updateFloatingCursor:FlutterFloatingCursorDragStateUpdate
+                                    withClient:_textInputClient
+                                  withPosition:@{@"X" : @(point.x), @"Y" : @(point.y)}];
 }

 - (void)endFloatingCursor {
-  [_textInputDelegate updateFloatingCursor:FlutterFloatingCursorDragStateEnd
-                                withClient:_textInputClient
-                              withPosition:@{@"X" : @(0), @"Y" : @(0)}];
+  [self.textInputDelegate updateFloatingCursor:FlutterFloatingCursorDragStateEnd
+                                    withClient:_textInputClient
+                                  withPosition:@{@"X" : @(0), @"Y" : @(0)}];
 }

 #pragma mark - UIKeyInput Overrides
@ -1139,9 +1162,11 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) {
  };

  if (_textInputClient == 0 && _autofillId != nil) {
-    [_textInputDelegate updateEditingClient:_textInputClient withState:state withTag:_autofillId];
+    [self.textInputDelegate updateEditingClient:_textInputClient
+                                      withState:state
+                                        withTag:_autofillId];
  } else {
-    [_textInputDelegate updateEditingClient:_textInputClient withState:state];
+    [self.textInputDelegate updateEditingClient:_textInputClient withState:state];
  }
 }

@ -1259,8 +1284,6 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) {
@end

@interface FlutterTextInputPlugin ()
-@property(nonatomic, strong) FlutterTextInputView* reusableInputView;
-
 // The current password-autofillable input fields that have yet to be saved.
@property(nonatomic, readonly)
    NSMutableDictionary<NSString*, FlutterTextInputView*>* autofillContext;
@ -1278,11 +1301,11 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) {
  self = [super init];

  if (self) {
-    _reusableInputView = [[FlutterTextInputView alloc] init];
-    _reusableInputView.secureTextEntry = NO;
    _autofillContext = [[NSMutableDictionary alloc] init];
-    _activeView = [_reusableInputView retain];
    _inputHider = [[FlutterTextInputViewAccessibilityHider alloc] init];
+    // Initialize activeView with a dummy view to keep tests
+    // passing.
+    _activeView = [[FlutterTextInputView alloc] init];
  }

  return self;
@ -1291,7 +1314,6 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) {
 - (void)dealloc {
  [self hideTextInput];
  _activeView.textInputDelegate = nil;
-  [_reusableInputView release];
  [_activeView release];
  [_inputHider release];
  [_autofillContext release];
@ -1398,14 +1420,14 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) {
  if (saveEntries) {
    // Make all the input fields in the autofill context visible,
    // then remove them to trigger autofill save.
-    [self cleanUpViewHierarchy:YES clearText:YES];
+    [self cleanUpViewHierarchy:YES clearText:YES delayRemoval:NO];
    [_autofillContext removeAllObjects];
    [self changeInputViewsAutofillVisibility:YES];
  } else {
    [_autofillContext removeAllObjects];
  }

-  [self cleanUpViewHierarchy:YES clearText:!saveEntries];
+  [self cleanUpViewHierarchy:YES clearText:!saveEntries delayRemoval:NO];
  [self addToInputParentViewIfNeeded:_activeView];
 }

@ -1414,9 +1436,11 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) {
  // Hide all input views from autofill, only make those in the new configuration visible
  // to autofill.
  [self changeInputViewsAutofillVisibility:NO];
+
+  // Update the current active view.
  switch (autofillTypeOf(configuration)) {
    case FlutterAutofillTypeNone:
-      self.activeView = [self updateAndShowReusableInputView:configuration];
+      self.activeView = [self createInputViewWith:configuration];
      break;
    case FlutterAutofillTypeRegular:
      // If the group does not involve password autofill, only install the
@ -1431,7 +1455,6 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) {
                                       isPasswordRelated:YES];
      break;
  }
-
  [_activeView setTextInputClient:client];
  [_activeView reloadInputViews];

@ -1441,27 +1464,29 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) {
  // them to free up resources and reduce the number of input views in the view
  // hierarchy.
  //
-  // This is scheduled on the runloop and delayed by 0.1s so we don't remove the
+  // The garbage views are decommissioned immediately, but the removeFromSuperview
+  // call is scheduled on the runloop and delayed by 0.1s so we don't remove the
  // text fields immediately (which seems to make the keyboard flicker).
  // See: https://github.com/flutter/flutter/issues/64628.
-  [self performSelector:@selector(collectGarbageInputViews) withObject:nil afterDelay:0.1];
+  [self cleanUpViewHierarchy:NO clearText:YES delayRemoval:YES];
 }

-// Updates and shows an input field that is not password related and has no autofill
-// hints. This method re-configures and reuses an existing instance of input field
-// instead of creating a new one.
-// Also updates the current autofill context.
- (FlutterTextInputView*)updateAndShowReusableInputView:(NSDictionary*)configuration {
+// Creates and shows an input field that is not password related and has no autofill
+// hints. This method returns a new FlutterTextInputView instance when called, since
+// UIKit uses the identity of `UITextInput` instances (or the identity of the input
+// views) to decide whether the IME's internal states should be reset. See:
+// https://github.com/flutter/flutter/issues/79031 .
+- (FlutterTextInputView*)createInputViewWith:(NSDictionary*)configuration {
  // It's possible that the configuration of this non-autofillable input view has
  // an autofill configuration without hints. If it does, remove it from the context.
  NSString* autofillId = autofillIdFromDictionary(configuration);
  if (autofillId) {
    [_autofillContext removeObjectForKey:autofillId];
  }
-
-  [_reusableInputView configureWithDictionary:configuration];
-  [self addToInputParentViewIfNeeded:_reusableInputView];
-  _reusableInputView.textInputDelegate = _textInputDelegate;
+  FlutterTextInputView* newView = [[FlutterTextInputView alloc] init];
+  [newView configureWithDictionary:configuration];
+  [self addToInputParentViewIfNeeded:newView];
+  newView.textInputDelegate = _textInputDelegate;

  for (NSDictionary* field in configuration[kAssociatedAutofillFields]) {
    NSString* autofillId = autofillIdFromDictionary(field);
@ -1469,7 +1494,7 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) {
      [_autofillContext removeObjectForKey:autofillId];
    }
  }
-  return _reusableInputView;
+  return [newView autorelease];
 }

 - (FlutterTextInputView*)updateAndShowAutofillViews:(NSArray*)fields
@ -1547,11 +1572,21 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) {
  return _inputHider.subviews;
 }

-// Removes every installed input field, unless it's in the current autofill
-// context. May remove the active view too if includeActiveView is YES.
+// Decommisions (See the "decommision" method on FlutterTextInputView) and removes
+// every installed input field, unless it's in the current autofill context.
+//
+// The active view will be decommisioned and removed from its superview too, if
+// includeActiveView is YES.
 // When clearText is YES, the text on the input fields will be set to empty before
 // they are removed from the view hierarchy, to avoid triggering autofill save.
- (void)cleanUpViewHierarchy:(BOOL)includeActiveView clearText:(BOOL)clearText {
+// If delayRemoval is true, removeFromSuperview will be scheduled on the runloop and
+// will be delayed by 0.1s so we don't remove the text fields immediately (which seems
+// to make the keyboard flicker).
+// See: https://github.com/flutter/flutter/issues/64628.
+
+- (void)cleanUpViewHierarchy:(BOOL)includeActiveView
+                   clearText:(BOOL)clearText
+                delayRemoval:(BOOL)delayRemoval {
  for (UIView* view in self.textInputViews) {
    if ([view isKindOfClass:[FlutterTextInputView class]] &&
        (includeActiveView || view != _activeView)) {
@ -1560,16 +1595,17 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) {
        if (clearText) {
          [inputView replaceRangeLocal:NSMakeRange(0, inputView.text.length) withText:@""];
        }
-        [view removeFromSuperview];
+        [inputView decommision];
+        if (delayRemoval) {
+          [inputView performSelector:@selector(removeFromSuperview) withObject:nil afterDelay:0.1];
+        } else {
+          [inputView removeFromSuperview];
+        }
      }
    }
  }
 }

- (void)collectGarbageInputViews {
-  [self cleanUpViewHierarchy:NO clearText:YES];
-}
-
 // Changes the visibility of every FlutterTextInputView currently in the
 // view hierarchy.
 - (void)changeInputViewsAutofillVisibility:(BOOL)newVisibility {
@ -1582,7 +1618,12 @@ static FlutterAutofillType autofillTypeOf(NSDictionary* configuration) {
 }

 // Resets the client id of every FlutterTextInputView in the view hierarchy
-// to 0. Called when a new text input connection will be established.
+// to 0.
+// Called before establishing a new text input connection.
+// For views in the current autofill context, they need to
+// stay in the view hierachy but should not be allowed to
+// send messages (other than autofill related ones) to the
+// framework.
 - (void)resetAllClientIds {
  for (UIView* view in self.textInputViews) {
    if ([view isKindOfClass:[FlutterTextInputView class]]) {
--- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.m
+++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.m
@ -19,6 +19,7 @@ FLUTTER_ASSERT_ARC
 - (void)setTextInputState:(NSDictionary*)state;
 - (void)setMarkedRect:(CGRect)markedRect;
 - (void)updateEditingState;
+- (void)decommisson;
 - (BOOL)isVisibleToAutofill;

@end
@ -56,7 +57,9 @@ FLUTTER_ASSERT_ARC
@property(nonatomic, readonly)
    NSMutableDictionary<NSString*, FlutterTextInputView*>* autofillContext;

- (void)collectGarbageInputViews;
+- (void)cleanUpViewHierarchy:(BOOL)includeActiveView
+                   clearText:(BOOL)clearText
+                delayRemoval:(BOOL)delayRemoval;
 - (NSArray<UIView*>*)textInputViews;
@end

@ -252,6 +255,34 @@ FLUTTER_ASSERT_ARC
  [activeView updateEditingState];
 }

+- (void)testDoNotReuseInputViews {
+  NSDictionary* config = self.mutableTemplateCopy;
+  [self setClientId:123 configuration:config];
+  FlutterTextInputView* currentView = textInputPlugin.activeView;
+  [self setClientId:456 configuration:config];
+
+  XCTAssertNotNil(currentView);
+  XCTAssertNotNil(textInputPlugin.activeView);
+  XCTAssertNotEqual(currentView, textInputPlugin.activeView);
+}
+
+- (void)testNoDanglingEnginePointer {
+  NSDictionary* config = self.mutableTemplateCopy;
+  [self setClientId:123 configuration:config];
+
+  // We'll hold onto the current view and try to access the engine
+  // after changing the active view.
+  FlutterTextInputView* currentView = textInputPlugin.activeView;
+  [self setClientId:456 configuration:config];
+  XCTAssertNotNil(currentView);
+  XCTAssertNotNil(textInputPlugin.activeView);
+  XCTAssertNotEqual(currentView, textInputPlugin.activeView);
+
+  // Verify that the view can no longer access the engine
+  // instance.
+  XCTAssertNil(currentView.textInputDelegate);
+}
+
 - (void)ensureOnlyActiveViewCanBecomeFirstResponder {
  for (FlutterTextInputView* inputView in self.installedInputViews) {
    XCTAssertEqual(inputView.canBecomeFirstResponder, inputView == textInputPlugin.activeView);
@ -566,7 +597,7 @@ FLUTTER_ASSERT_ARC

  XCTAssertEqual(textInputPlugin.autofillContext.count, 2);

-  [textInputPlugin collectGarbageInputViews];
+  [textInputPlugin cleanUpViewHierarchy:NO clearText:YES delayRemoval:NO];
  XCTAssertEqual(self.installedInputViews.count, 2);
  XCTAssertEqual(textInputPlugin.textInputView, textInputPlugin.autofillContext[@"field1"]);
  [self ensureOnlyActiveViewCanBecomeFirstResponder];
@ -589,7 +620,7 @@ FLUTTER_ASSERT_ARC
  XCTAssertEqual(self.viewsVisibleToAutofill.count, 2);
  XCTAssertEqual(textInputPlugin.autofillContext.count, 3);

-  [textInputPlugin collectGarbageInputViews];
+  [textInputPlugin cleanUpViewHierarchy:NO clearText:YES delayRemoval:NO];
  XCTAssertEqual(self.installedInputViews.count, 3);
  XCTAssertEqual(textInputPlugin.textInputView, textInputPlugin.autofillContext[@"field1"]);
  [self ensureOnlyActiveViewCanBecomeFirstResponder];
@ -609,7 +640,7 @@ FLUTTER_ASSERT_ARC
  XCTAssertEqual(self.viewsVisibleToAutofill.count, 1);
  XCTAssertEqual(textInputPlugin.autofillContext.count, 3);

-  [textInputPlugin collectGarbageInputViews];
+  [textInputPlugin cleanUpViewHierarchy:NO clearText:YES delayRemoval:NO];
  XCTAssertEqual(self.installedInputViews.count, 4);

  // Old autofill input fields are still installed and reused.
@ -628,7 +659,7 @@ FLUTTER_ASSERT_ARC
  XCTAssertEqual(self.viewsVisibleToAutofill.count, 1);
  XCTAssertEqual(textInputPlugin.autofillContext.count, 3);

-  [textInputPlugin collectGarbageInputViews];
+  [textInputPlugin cleanUpViewHierarchy:NO clearText:YES delayRemoval:NO];
  XCTAssertEqual(self.installedInputViews.count, 4);

  // Old autofill input fields are still installed and reused.
@ -673,7 +704,6 @@ FLUTTER_ASSERT_ARC
  [self ensureOnlyActiveViewCanBecomeFirstResponder];

  [self commitAutofillContextAndVerify];
-  XCTAssertNotEqual(textInputPlugin.textInputView, textInputPlugin.reusableInputView);
  [self ensureOnlyActiveViewCanBecomeFirstResponder];

  // Install the password field again.
@ -682,31 +712,28 @@ FLUTTER_ASSERT_ARC
  [self setClientId:124 configuration:field3];
  XCTAssertEqual(self.viewsVisibleToAutofill.count, 1);

-  [textInputPlugin collectGarbageInputViews];
+  [textInputPlugin cleanUpViewHierarchy:NO clearText:YES delayRemoval:NO];
  XCTAssertEqual(self.installedInputViews.count, 3);
  XCTAssertEqual(textInputPlugin.autofillContext.count, 2);
  XCTAssertNotEqual(textInputPlugin.textInputView, nil);
  [self ensureOnlyActiveViewCanBecomeFirstResponder];

  [self commitAutofillContextAndVerify];
-  XCTAssertNotEqual(textInputPlugin.textInputView, textInputPlugin.reusableInputView);
  [self ensureOnlyActiveViewCanBecomeFirstResponder];

  // Now switch to an input field that does not autofill.
  [self setClientId:125 configuration:self.mutableTemplateCopy];

  XCTAssertEqual(self.viewsVisibleToAutofill.count, 0);
-  XCTAssertEqual(textInputPlugin.textInputView, textInputPlugin.reusableInputView);
  // The active view should still be installed so it doesn't get
  // deallocated.

-  [textInputPlugin collectGarbageInputViews];
+  [textInputPlugin cleanUpViewHierarchy:NO clearText:YES delayRemoval:NO];
  XCTAssertEqual(self.installedInputViews.count, 1);
  XCTAssertEqual(textInputPlugin.autofillContext.count, 0);
  [self ensureOnlyActiveViewCanBecomeFirstResponder];

  [self commitAutofillContextAndVerify];
-  XCTAssertEqual(textInputPlugin.textInputView, textInputPlugin.reusableInputView);
  [self ensureOnlyActiveViewCanBecomeFirstResponder];
 }

@ -798,7 +825,7 @@ FLUTTER_ASSERT_ARC

  XCTAssertEqual(self.installedInputViews.count, 2);

-  [textInputPlugin collectGarbageInputViews];
+  [textInputPlugin cleanUpViewHierarchy:NO clearText:YES delayRemoval:NO];
  XCTAssertEqual(self.installedInputViews.count, 1);

  // Verify the old input view is properly cleaned up.