Allow for whitelisted flags to be passed to the Dart VM (#9148)

bkonyi · web-flow · commit 12f48f760159 · 2019-06-04T14:51:29.000-07:00
* Allow for whitelisted flags to be passed to the Dart VM Fixed part of flutter/flutter#32176
diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc
@@ -8,13 +8,15 @@
 #include <future>
 #include <memory>
 
+#include "flutter/fml/command_line.h"
 #include "flutter/fml/make_copyable.h"
 #include "flutter/fml/message_loop.h"
 #include "flutter/fml/synchronization/count_down_latch.h"
 #include "flutter/fml/synchronization/waitable_event.h"
 #include "flutter/shell/common/platform_view.h"
 #include "flutter/shell/common/rasterizer.h"
 #include "flutter/shell/common/shell_test.h"
+#include "flutter/shell/common/switches.h"
 #include "flutter/shell/common/thread_host.h"
 #include "flutter/testing/testing.h"
 
@@ -274,5 +276,42 @@ TEST_F(ShellTest, SecondaryIsolateBindingsAreSetupViaShellSettings) {
   ASSERT_FALSE(DartVMRef::IsInstanceRunning());
 }
 
+TEST_F(ShellTest, BlacklistedDartVMFlag) {
+  // Run this test in a thread-safe manner, otherwise gtest will complain.
+  ::testing::FLAGS_gtest_death_test_style = "threadsafe";
+
+  const std::vector<fml::CommandLine::Option> options = {
+      fml::CommandLine::Option("dart-flags", "--verify_after_gc")};
+  fml::CommandLine command_line("", options, std::vector<std::string>());
+
+#if FLUTTER_RUNTIME_MODE != FLUTTER_RUNTIME_MODE_RELEASE && \
+    FLUTTER_RUNTIME_MODE != FLUTTER_RUNTIME_MODE_DYNAMIC_RELEASE
+  // Upon encountering a non-whitelisted Dart flag the process terminates.
+  const char* expected =
+      "Encountered blacklisted Dart VM flag: --verify_after_gc";
+  ASSERT_DEATH(flutter::SettingsFromCommandLine(command_line), expected);
+#else
+  flutter::Settings settings = flutter::SettingsFromCommandLine(command_line);
+  EXPECT_EQ(settings.dart_flags.size(), 0u);
+#endif
+}
+
+TEST_F(ShellTest, WhitelistedDartVMFlag) {
+  const std::vector<fml::CommandLine::Option> options = {
+      fml::CommandLine::Option("dart-flags",
+                               "--max_profile_depth 1,--trace_service")};
+  fml::CommandLine command_line("", options, std::vector<std::string>());
+  flutter::Settings settings = flutter::SettingsFromCommandLine(command_line);
+
+#if FLUTTER_RUNTIME_MODE != FLUTTER_RUNTIME_MODE_RELEASE && \
+    FLUTTER_RUNTIME_MODE != FLUTTER_RUNTIME_MODE_DYNAMIC_RELEASE
+  EXPECT_EQ(settings.dart_flags.size(), 2u);
+  EXPECT_EQ(settings.dart_flags[0], "--max_profile_depth 1");
+  EXPECT_EQ(settings.dart_flags[1], "--trace_service");
+#else
+  EXPECT_EQ(settings.dart_flags.size(), 0u);
+#endif
+}
+
 }  // namespace testing
 }  // namespace flutter
diff --git a/shell/common/switches.cc b/shell/common/switches.cc
@@ -11,6 +11,7 @@
 
 #include "flutter/fml/native_library.h"
 #include "flutter/fml/paths.h"
+#include "flutter/fml/size.h"
 #include "flutter/fml/string_view.h"
 #include "flutter/shell/version/version.h"
 
@@ -36,6 +37,18 @@ struct SwitchDesc {
 #define DEF_SWITCHES_END };
 // clang-format on
 
+#if FLUTTER_RUNTIME_MODE != FLUTTER_RUNTIME_MODE_RELEASE && \
+    FLUTTER_RUNTIME_MODE != FLUTTER_RUNTIME_MODE_DYNAMIC_RELEASE
+
+// List of common and safe VM flags to allow to be passed directly to the VM.
+static const std::string gDartFlagsWhitelist[] = {
+    "--max_profile_depth",     "--profile_period",         "--random_seed",
+    "--trace_profiler",        "--trace_profiler_verbose", "--trace_service",
+    "--trace_service_verbose",
+};
+
+#endif
+
 // Include again for struct definition.
 #include "flutter/shell/common/switches.h"
 
@@ -102,6 +115,24 @@ const fml::StringView FlagForSwitch(Switch swtch) {
   return fml::StringView();
 }
 
+#if FLUTTER_RUNTIME_MODE != FLUTTER_RUNTIME_MODE_RELEASE && \
+    FLUTTER_RUNTIME_MODE != FLUTTER_RUNTIME_MODE_DYNAMIC_RELEASE
+
+static bool IsWhitelistedDartVMFlag(const std::string& flag) {
+  for (uint32_t i = 0; i < fml::size(gDartFlagsWhitelist); ++i) {
+    const std::string& allowed = gDartFlagsWhitelist[i];
+    // Check that the prefix of the flag matches one of the whitelisted flags.
+    // We don't need to worry about cases like "--safe --sneaky_dangerous" as
+    // the VM will discard these as a single unrecognized flag.
+    if (std::equal(allowed.begin(), allowed.end(), flag.begin())) {
+      return true;
+    }
+  }
+  return false;
+}
+
+#endif
+
 template <typename T>
 static bool GetSwitchValue(const fml::CommandLine& command_line,
                            Switch sw,
@@ -260,18 +291,24 @@ Settings SettingsFromCommandLine(const fml::CommandLine& command_line) {
   settings.use_test_fonts =
       command_line.HasOption(FlagForSwitch(Switch::UseTestFonts));
 
+#if FLUTTER_RUNTIME_MODE != FLUTTER_RUNTIME_MODE_RELEASE && \
+    FLUTTER_RUNTIME_MODE != FLUTTER_RUNTIME_MODE_DYNAMIC_RELEASE
   command_line.GetOptionValue(FlagForSwitch(Switch::LogTag), &settings.log_tag);
   std::string all_dart_flags;
   if (command_line.GetOptionValue(FlagForSwitch(Switch::DartFlags),
                                   &all_dart_flags)) {
     std::stringstream stream(all_dart_flags);
-    std::istream_iterator<std::string> end;
-    for (std::istream_iterator<std::string> it(stream); it != end; ++it)
-      settings.dart_flags.push_back(*it);
+    std::string flag;
+
+    // Assume that individual flags are comma separated.
+    while (std::getline(stream, flag, ',')) {
+      if (!IsWhitelistedDartVMFlag(flag)) {
+        FML_LOG(FATAL) << "Encountered blacklisted Dart VM flag: " << flag;
+      }
+      settings.dart_flags.push_back(flag);
+    }
   }
 
-#if FLUTTER_RUNTIME_MODE != FLUTTER_RUNTIME_MODE_RELEASE && \
-    FLUTTER_RUNTIME_MODE != FLUTTER_RUNTIME_MODE_DYNAMIC_RELEASE
   settings.trace_skia =
       command_line.HasOption(FlagForSwitch(Switch::TraceSkia));
   settings.trace_systrace =
diff --git a/shell/platform/android/io/flutter/app/FlutterActivityDelegate.java b/shell/platform/android/io/flutter/app/FlutterActivityDelegate.java
@@ -312,6 +312,13 @@ private static String[] getArgsFromIntent(Intent intent) {
         if (intent.getBooleanExtra("verbose-logging", false)) {
             args.add("--verbose-logging");
         }
+        // NOTE: all flags provided with this argument are subject to filtering
+        // based on a whitelist in shell/common/switches.cc. If any flag provided
+        // is not present in the whitelist, the process will immediately
+        // terminate.
+        if (intent.hasExtra("dart-flags")) {
+            args.add("--dart-flags=" + intent.getStringExtra("dart-flags"));
+        }
         if (!args.isEmpty()) {
             String[] argsArray = new String[args.size()];
             return args.toArray(argsArray);
