[macOS] support secure restorable state by default (flutter#151605)

cbracken · pull[bot] · commit 44e0d32cd3e7 · 2024-08-28T01:53:30.000Z
By default, Flutter apps only do default AppKit app serialisation of Window location etc. and by default, state serialisation in AppKit apps is compatible with `NSSecureCoding`. AppKit apps generated since Xcode 13.2 include this method in the app delegate generated by the default app template. Background ========== This method was added to opt into having [de]serialization require a coder implementing the `NSSecureCoding` protocol. Apple wasn't able to force this across the board, because `NSSecureCoding` limits certain behaviours during deserialisation, which some third-party apps have have previously relied on. Specific background on the sorts of vulnerabilities that `NSSecureCoding` was designed to prevent are described in the `NSSecureCoding` documentation: https://developer.apple.com/documentation/foundation/nssecurecoding?language=objc A demonstration of a root privilege escalation and SIP bypass vulnerability is described in the following blog post: https://sector7.computest.nl/post/2022-08-process-injection-breaking-all-macos-security-layers-with-a-single-vulnerability/ Fixes: flutter#150062 ## Pre-launch Checklist - [X] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [X] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [X] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [X] I signed the [CLA]. - [X] I listed at least one issue that this PR fixes in the description above. - [X] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [X] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [X] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord].  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
diff --git a/dev/a11y_assessments/macos/Runner/AppDelegate.swift b/dev/a11y_assessments/macos/Runner/AppDelegate.swift
@@ -10,4 +10,8 @@ class AppDelegate: FlutterAppDelegate {
   override func applicationShouldTerminateAfterLastWindowClosed(_ sender: NSApplication) -> Bool {
     return true
   }
+
+  override func applicationSupportsSecureRestorableState(_ app: NSApplication) -> Bool {
+    return true
+  }
 }
diff --git a/dev/benchmarks/complex_layout/macos/Runner/AppDelegate.swift b/dev/benchmarks/complex_layout/macos/Runner/AppDelegate.swift
@@ -10,4 +10,8 @@ class AppDelegate: FlutterAppDelegate {
   override func applicationShouldTerminateAfterLastWindowClosed(_ sender: NSApplication) -> Bool {
     return true
   }
+
+  override func applicationSupportsSecureRestorableState(_ app: NSApplication) -> Bool {
+    return true
+  }
 }
diff --git a/dev/benchmarks/macrobenchmarks/macos/Runner/AppDelegate.swift b/dev/benchmarks/macrobenchmarks/macos/Runner/AppDelegate.swift
@@ -10,4 +10,8 @@ class AppDelegate: FlutterAppDelegate {
   override func applicationShouldTerminateAfterLastWindowClosed(_ sender: NSApplication) -> Bool {
     return true
   }
+
+  override func applicationSupportsSecureRestorableState(_ app: NSApplication) -> Bool {
+    return true
+  }
 }
diff --git a/dev/integration_tests/channels/macos/Runner/AppDelegate.swift b/dev/integration_tests/channels/macos/Runner/AppDelegate.swift
@@ -10,4 +10,8 @@ class AppDelegate: FlutterAppDelegate {
   override func applicationShouldTerminateAfterLastWindowClosed(_ sender: NSApplication) -> Bool {
     return true
   }
+
+  override func applicationSupportsSecureRestorableState(_ app: NSApplication) -> Bool {
+    return true
+  }
 }
diff --git a/dev/integration_tests/flavors/macos/Runner/AppDelegate.swift b/dev/integration_tests/flavors/macos/Runner/AppDelegate.swift
@@ -10,4 +10,8 @@ class AppDelegate: FlutterAppDelegate {
   override func applicationShouldTerminateAfterLastWindowClosed(_ sender: NSApplication) -> Bool {
     return true
   }
+
+  override func applicationSupportsSecureRestorableState(_ app: NSApplication) -> Bool {
+    return true
+  }
 }
diff --git a/dev/integration_tests/flutter_gallery/macos/Runner/AppDelegate.swift b/dev/integration_tests/flutter_gallery/macos/Runner/AppDelegate.swift
@@ -10,4 +10,8 @@ class AppDelegate: FlutterAppDelegate {
   override func applicationShouldTerminateAfterLastWindowClosed(_ sender: NSApplication) -> Bool {
     return true
   }
+
+  override func applicationSupportsSecureRestorableState(_ app: NSApplication) -> Bool {
+    return true
+  }
 }
diff --git a/dev/integration_tests/ui/macos/Runner/AppDelegate.swift b/dev/integration_tests/ui/macos/Runner/AppDelegate.swift
@@ -10,4 +10,8 @@ class AppDelegate: FlutterAppDelegate {
   override func applicationShouldTerminateAfterLastWindowClosed(_ sender: NSApplication) -> Bool {
     return true
   }
+
+  override func applicationSupportsSecureRestorableState(_ app: NSApplication) -> Bool {
+    return true
+  }
 }
diff --git a/dev/manual_tests/macos/Runner/AppDelegate.swift b/dev/manual_tests/macos/Runner/AppDelegate.swift
@@ -10,4 +10,8 @@ class AppDelegate: FlutterAppDelegate {
   override func applicationShouldTerminateAfterLastWindowClosed(_ sender: NSApplication) -> Bool {
     return true
   }
+
+  override func applicationSupportsSecureRestorableState(_ app: NSApplication) -> Bool {
+    return true
+  }
 }
diff --git a/examples/api/macos/Runner/AppDelegate.swift b/examples/api/macos/Runner/AppDelegate.swift
@@ -10,4 +10,8 @@ class AppDelegate: FlutterAppDelegate {
   override func applicationShouldTerminateAfterLastWindowClosed(_ sender: NSApplication) -> Bool {
     return true
   }
+
+  override func applicationSupportsSecureRestorableState(_ app: NSApplication) -> Bool {
+    return true
+  }
 }
diff --git a/examples/flutter_view/macos/Runner/AppDelegate.swift b/examples/flutter_view/macos/Runner/AppDelegate.swift
@@ -10,4 +10,8 @@ class AppDelegate: FlutterAppDelegate {
   override func applicationShouldTerminateAfterLastWindowClosed(_ sender: NSApplication) -> Bool {
     return true
   }
+
+  override func applicationSupportsSecureRestorableState(_ app: NSApplication) -> Bool {
+    return true
+  }
 }
diff --git a/examples/hello_world/macos/Runner/AppDelegate.swift b/examples/hello_world/macos/Runner/AppDelegate.swift
@@ -10,4 +10,8 @@ class AppDelegate: FlutterAppDelegate {
   override func applicationShouldTerminateAfterLastWindowClosed(_ sender: NSApplication) -> Bool {
     return true
   }
+
+  override func applicationSupportsSecureRestorableState(_ app: NSApplication) -> Bool {
+    return true
+  }
 }
diff --git a/examples/image_list/macos/Runner/AppDelegate.swift b/examples/image_list/macos/Runner/AppDelegate.swift
@@ -10,4 +10,8 @@ class AppDelegate: FlutterAppDelegate {
   override func applicationShouldTerminateAfterLastWindowClosed(_ sender: NSApplication) -> Bool {
     return true
   }
+
+  override func applicationSupportsSecureRestorableState(_ app: NSApplication) -> Bool {
+    return true
+  }
 }
diff --git a/examples/layers/macos/Runner/AppDelegate.swift b/examples/layers/macos/Runner/AppDelegate.swift
@@ -10,4 +10,8 @@ class AppDelegate: FlutterAppDelegate {
   override func applicationShouldTerminateAfterLastWindowClosed(_ sender: NSApplication) -> Bool {
     return true
   }
+
+  override func applicationSupportsSecureRestorableState(_ app: NSApplication) -> Bool {
+    return true
+  }
 }
diff --git a/examples/platform_channel/macos/Runner/AppDelegate.swift b/examples/platform_channel/macos/Runner/AppDelegate.swift
@@ -10,4 +10,8 @@ class AppDelegate: FlutterAppDelegate {
   override func applicationShouldTerminateAfterLastWindowClosed(_ sender: NSApplication) -> Bool {
     return true
   }
+
+  override func applicationSupportsSecureRestorableState(_ app: NSApplication) -> Bool {
+    return true
+  }
 }
diff --git a/examples/platform_view/macos/Runner/AppDelegate.swift b/examples/platform_view/macos/Runner/AppDelegate.swift
@@ -10,4 +10,8 @@ class AppDelegate: FlutterAppDelegate {
   override func applicationShouldTerminateAfterLastWindowClosed(_ sender: NSApplication) -> Bool {
     return true
   }
+
+  override func applicationSupportsSecureRestorableState(_ app: NSApplication) -> Bool {
+    return true
+  }
 }
diff --git a/packages/flutter_tools/lib/src/macos/build_macos.dart b/packages/flutter_tools/lib/src/macos/build_macos.dart
@@ -27,6 +27,7 @@ import 'migrations/flutter_application_migration.dart';
 import 'migrations/macos_deployment_target_migration.dart';
 import 'migrations/nsapplicationmain_deprecation_migration.dart';
 import 'migrations/remove_macos_framework_link_and_embedding_migration.dart';
+import 'migrations/secure_restorable_state_migration.dart';
 
 /// When run in -quiet mode, Xcode should only print from the underlying tasks to stdout.
 /// Passing this regexp to trace moves the stdout output to stderr.
@@ -87,6 +88,7 @@ Future<void> buildMacOS({
     XcodeThinBinaryBuildPhaseInputPathsMigration(flutterProject.macos, globals.logger),
     FlutterApplicationMigration(flutterProject.macos, globals.logger),
     NSApplicationMainDeprecationMigration(flutterProject.macos, globals.logger),
+    SecureRestorableStateMigration(flutterProject.macos, globals.logger),
     if (flutterProject.usesSwiftPackageManager && flutterProject.macos.flutterPluginSwiftPackageManifest.existsSync())
       SwiftPackageManagerIntegrationMigration(
         flutterProject.macos,
diff --git a/packages/flutter_tools/lib/src/macos/migrations/secure_restorable_state_migration.dart b/packages/flutter_tools/lib/src/macos/migrations/secure_restorable_state_migration.dart
@@ -0,0 +1,88 @@
+// Copyright 2014 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import '../../base/file_system.dart';
+import '../../base/project_migrator.dart';
+import '../../xcode_project.dart';
+
+const String _appDelegateFileBefore = r'''
+class AppDelegate: FlutterAppDelegate {
+  override func applicationShouldTerminateAfterLastWindowClosed(_ sender: NSApplication) -> Bool {
+    return true
+  }
+}''';
+
+const String _appDelegateFileAfter = r'''
+class AppDelegate: FlutterAppDelegate {
+  override func applicationShouldTerminateAfterLastWindowClosed(_ sender: NSApplication) -> Bool {
+    return true
+  }
+
+  override func applicationSupportsSecureRestorableState(_ app: NSApplication) -> Bool {
+    return true
+  }
+}''';
+
+/// Add `applicationSupportsSecureRestorableState` if not already present.
+///
+/// In all new AppKit apps since Xcode 13.2, the AppDelegate template includes
+/// this method, which opts in to requiring safe deserialization via the
+/// `NSSecureCoding` protocol. Because this required new API, existing apps
+/// need to opt in to this behavior.
+///
+/// Since nearly all Flutter macOS apps will be doing serialization of Flutter
+/// state via Dart code, it's a very safe bet that the vast majority of
+/// existing Flutter apps can safely enable this flag. The few apps that
+/// are doing serialization via older insecure APIs can update the migrated
+/// code to return false.
+///
+/// See:
+/// https://developer.apple.com/documentation/foundation/nssecurecoding?language=objc
+class SecureRestorableStateMigration extends ProjectMigrator {
+  SecureRestorableStateMigration(
+    MacOSProject project,
+    super.logger,
+  ) : _appDelegateSwift = project.appDelegateSwift;
+
+  final File _appDelegateSwift;
+
+  @override
+  Future<void> migrate() async {
+    // Skip this migration if the project uses Objective-C.
+    if (!_appDelegateSwift.existsSync()) {
+      logger.printTrace(
+        'macos/Runner/AppDelegate.swift not found. Skipping applicationSupportsSecureRestorableState migration.'
+      );
+      return;
+    }
+    final String original = _appDelegateSwift.readAsStringSync();
+
+    // If we have an AppDelegate.swift, but can't migrate, log a warning.
+    if (!original.contains(_appDelegateFileBefore)) {
+      if (original.contains('applicationSupportsSecureRestorableState')) {
+        // User has already overridden this method. Exit quietly.
+        return;
+      }
+
+      logger.printWarning('''
+macos/Runner/AppDelegate.swift has been modified and cannot be automatically migrated.
+We recommend developers override applicationSupportsSecureRestorableState in AppDelegate.swift as follows:
+override func applicationSupportsSecureRestorableState(_ app: NSApplication) -> Bool {
+  return true
+}
+''');
+    }
+
+    // Migrate the macos/Runner/AppDelegate.swift file.
+    final String migrated = original.replaceFirst(_appDelegateFileBefore, _appDelegateFileAfter);
+    if (original == migrated) {
+      return;
+    }
+
+    logger.printWarning(
+      'macos/Runner/AppDelegate.swift does not override applicationSupportsSecureRestorableState. Updating.'
+    );
+    _appDelegateSwift.writeAsStringSync(migrated);
+  }
+}
diff --git a/packages/flutter_tools/templates/app_shared/macos.tmpl/Runner/AppDelegate.swift b/packages/flutter_tools/templates/app_shared/macos.tmpl/Runner/AppDelegate.swift
@@ -6,4 +6,8 @@ class AppDelegate: FlutterAppDelegate {
   override func applicationShouldTerminateAfterLastWindowClosed(_ sender: NSApplication) -> Bool {
     return true
   }
+
+  override func applicationSupportsSecureRestorableState(_ app: NSApplication) -> Bool {
+    return true
+  }
 }
diff --git a/packages/flutter_tools/test/general.shard/macos/macos_project_migration_test.dart b/packages/flutter_tools/test/general.shard/macos/macos_project_migration_test.dart

Original file line number	Diff line number	Diff line change
`@@ -10,4 +10,8 @@ class AppDelegate: FlutterAppDelegate {`
`10`	`10`	`override func applicationShouldTerminateAfterLastWindowClosed(_ sender: NSApplication) -> Bool {`
`11`	`11`	`return true`
`12`	`12`	`}`
	`13`	`+`
	`14`	`+ override func applicationSupportsSecureRestorableState(_ app: NSApplication) -> Bool {`
	`15`	`+ return true`
	`16`	`+ }`
`13`	`17`	`}`