Bug 1570738 - Record Telemetry if eval() is used in the Parent Process r=ckerschb

Differential Revision: https://phabricator.services.mozilla.com/D40332

--HG--
extra : moz-landing-system : lando

browser/app/profile/firefox.js

@@ -500,9 +500,10 @@ pref("browser.tabs.remote.enforceRemoteTypeRestrictions", true);
 #endif
 
 #ifdef NIGHTLY_BUILD
-// allow_eval_with_system_principal is enabled on Firefox Desktop only at this
+// allow_eval_* is enabled on Firefox Desktop only at this
 // point in time
 pref("security.allow_eval_with_system_principal", false);
+pref("security.allow_eval_in_parent_process", false);
 pref("browser.tabs.remote.useHTTPResponseProcessSelection", true);
 #else
 // Disabled outside of nightly due to bug 1554217

caps/nsScriptSecurityManager.cpp

@@ -424,11 +424,12 @@ bool nsScriptSecurityManager::ContentSecurityPolicyPermitsJSAction(
   nsresult rv = csp->GetAllowsEval(&reportViolation, &evalOK);
 
   // A little convoluted. We want the scriptSample for a) reporting a violation
-  // or b) passing it to AssertEvalNotUsingSystemPrincipal. So do the work to
-  // get it if either of those cases is true.
+  // or b) passing it to AssertEvalNotUsingSystemPrincipal or c) we're in the
+  // parent process. So do the work to get it if either of those cases is true.
   nsAutoJSString scriptSample;
   nsCOMPtr<nsIPrincipal> subjectPrincipal = nsContentUtils::SubjectPrincipal();
-  if (reportViolation || subjectPrincipal->IsSystemPrincipal()) {
+  if (reportViolation || subjectPrincipal->IsSystemPrincipal() ||
+      XRE_IsE10sParentProcess()) {
     JS::Rooted<JSString*> jsString(cx, JS::ToString(cx, aValue));
     if (NS_WARN_IF(!jsString)) {
       JS_ClearPendingException(cx);
@@ -442,8 +443,8 @@ bool nsScriptSecurityManager::ContentSecurityPolicyPermitsJSAction(
   }
 
 #if !defined(ANDROID) && (defined(NIGHTLY_BUILD) || defined(DEBUG))
-  nsContentSecurityManager::AssertEvalNotUsingSystemPrincipal(
-      cx, subjectPrincipal, scriptSample);
+  nsContentSecurityManager::AssertEvalNotRestricted(cx, subjectPrincipal,
+                                                    scriptSample);
 #endif
 
   if (NS_FAILED(rv)) {

dom/security/CSPEvalChecker.cpp

@@ -33,8 +33,8 @@ nsresult CheckInternal(nsIContentSecurityPolicy* aCSP,
 
 #if !defined(ANDROID) && (defined(NIGHTLY_BUILD) || defined(DEBUG))
   JSContext* cx = nsContentUtils::GetCurrentJSContext();
-  nsContentSecurityManager::AssertEvalNotUsingSystemPrincipal(
-      cx, aSubjectPrincipal, aExpression);
+  nsContentSecurityManager::AssertEvalNotRestricted(cx, aSubjectPrincipal,
+                                                    aExpression);
 #endif
 
   // The value is set at any "return", but better to have a default value here.

dom/security/nsContentSecurityManager.cpp

@@ -71,6 +71,8 @@ static NS_NAMED_LITERAL_STRING(sAllowedEval1, "this");
 static NS_NAMED_LITERAL_STRING(sAllowedEval2,
                                "function anonymous(\n) {\nreturn this\n}");
 
+static Atomic<bool, mozilla::Relaxed> sTelemetryEventEnabled(false);
+
 /* static */
 bool nsContentSecurityManager::AllowTopLevelNavigationToDataURI(
     nsIChannel* aChannel) {
@@ -384,20 +386,32 @@ FilenameType nsContentSecurityManager::FilenameToEvalType(
 }
 
 /* static */
-void nsContentSecurityManager::AssertEvalNotUsingSystemPrincipal(
+void nsContentSecurityManager::AssertEvalNotRestricted(
     JSContext* cx, nsIPrincipal* aSubjectPrincipal, const nsAString& aScript) {
-  if (!aSubjectPrincipal->IsSystemPrincipal()) {
+  bool systemPrincipal = aSubjectPrincipal->IsSystemPrincipal();
+  if (systemPrincipal &&
+      StaticPrefs::security_allow_eval_with_system_principal()) {
+    MOZ_LOG(
+        sCSMLog, LogLevel::Debug,
+        ("Allowing eval() %s because allowing pref is "
+         "enabled",
+         (systemPrincipal ? "with System Principal" : "in parent process")));
     return;
   }
 
-  // Use static pref for performance reasons.
-  if (StaticPrefs::security_allow_eval_with_system_principal()) {
+  if (XRE_IsE10sParentProcess() &&
+      StaticPrefs::security_allow_eval_in_parent_process()) {
     MOZ_LOG(sCSMLog, LogLevel::Debug,
-            ("Allowing eval() with SystemPrincipal because allowing pref is "
+            ("Allowing eval() in parent process because allowing pref is "
              "enabled"));
     return;
   }
 
+  if (!systemPrincipal && !XRE_IsE10sParentProcess()) {
+    // Usage of eval we are unconcerned with.
+    return;
+  }
+
   // This preferences is a file used for autoconfiguration of Firefox
   // by administrators. It has also been (ab)used by the userChromeJS
   // project to run legacy-style 'extensions', some of which use eval,
@@ -405,43 +419,63 @@ void nsContentSecurityManager::AssertEvalNotUsingSystemPrincipal(
   nsAutoString configPref;
   Preferences::GetString("general.config.filename", configPref);
   if (!configPref.IsEmpty()) {
-    MOZ_LOG(sCSMLog, LogLevel::Debug,
-            ("Allowing eval() with SystemPrincipal because of "
-             "general.config.filename"));
+    MOZ_LOG(
+        sCSMLog, LogLevel::Debug,
+        ("Allowing eval() %s because of "
+         "general.config.filename",
+         (systemPrincipal ? "with System Principal" : "in parent process")));
     return;
   }
 
   // We permit these two common idioms to get access to the global JS object
   if (!aScript.IsEmpty() &&
       (aScript == sAllowedEval1 || aScript == sAllowedEval2)) {
-    MOZ_LOG(sCSMLog, LogLevel::Debug,
-            ("Allowing eval() with SystemPrincipal because a key string is "
-             "provided"));
+    MOZ_LOG(
+        sCSMLog, LogLevel::Debug,
+        ("Allowing eval() %s because a key string is "
+         "provided",
+         (systemPrincipal ? "with System Principal" : "in parent process")));
     return;
   }
 
-  nsAutoCString fileName;
-  JS::AutoFilename scriptFilename;
-  if (JS::DescribeScriptedCaller(cx, &scriptFilename)) {
-    nsDependentCSubstring fileName_(scriptFilename.get(),
-                                    strlen(scriptFilename.get()));
-    ToLowerCase(fileName_);
-    // Extract file name alone if scriptFilename contains line number
-    // separated by multiple space delimiters in few cases.
-    int32_t fileNameIndex = fileName_.FindChar(' ');
-    if (fileNameIndex != -1) {
-      fileName_.SetLength(fileNameIndex);
-    }
-
-    for (const nsLiteralCString& allowlistEntry : evalAllowlist) {
-      if (fileName_.Equals(allowlistEntry)) {
-        return;
+  // Check the allowlist for the provided filename. getFilename is a helper
+  // function
+  auto getFilename = [](JSContext* cx) -> const nsCString {
+    JS::AutoFilename scriptFilename;
+    if (JS::DescribeScriptedCaller(cx, &scriptFilename)) {
+      nsDependentCSubstring fileName_(scriptFilename.get(),
+                                      strlen(scriptFilename.get()));
+      ToLowerCase(fileName_);
+      // Extract file name alone if scriptFilename contains line number
+      // separated by multiple space delimiters in few cases.
+      int32_t fileNameIndex = fileName_.FindChar(' ');
+      if (fileNameIndex != -1) {
+        fileName_.SetLength(fileNameIndex);
       }
-    }
 
-    fileName = fileName_;
+      nsAutoCString fileName(fileName_);
+      return std::move(fileName);
+    }
+    return NS_LITERAL_CSTRING("unknown-file");
+  };
+
+  nsCString fileName = getFilename(cx);
+  for (const nsLiteralCString& allowlistEntry : evalAllowlist) {
+    if (fileName.Equals(allowlistEntry)) {
+      MOZ_LOG(
+          sCSMLog, LogLevel::Debug,
+          ("Allowing eval() %s because the containing "
+           "file is in the allowlist",
+           (systemPrincipal ? "with System Principal" : "in parent process")));
+      return;
+    }
   }
 
+  // Send Telemetry
+  Telemetry::EventID eventType =
+      systemPrincipal ? Telemetry::EventID::Security_Evalusage_Systemcontext
+                      : Telemetry::EventID::Security_Evalusage_Parentprocess;
+
   FilenameType fileNameType =
       FilenameToEvalType(NS_ConvertUTF8toUTF16(fileName));
   mozilla::Maybe<nsTArray<EventExtraEntry>> extra;
@@ -452,18 +486,25 @@ void nsContentSecurityManager::AssertEvalNotUsingSystemPrincipal(
   } else {
     extra = Nothing();
   }
-  Telemetry::RecordEvent(Telemetry::EventID::Security_Evalusage_Systemcontext,
-                         mozilla::Some(fileNameType.first()), extra);
+  if (!sTelemetryEventEnabled.exchange(true)) {
+    sTelemetryEventEnabled = true;
+    Telemetry::SetEventRecordingEnabled(
+        NS_LITERAL_CSTRING("security.evalUsage"), true);
+  }
+  Telemetry::RecordEvent(eventType, mozilla::Some(fileNameType.first()), extra);
 
+  // Crash or Log
 #ifdef DEBUG
   MOZ_CRASH_UNSAFE_PRINTF(
-      "Blocking eval() with SystemPrincipal from file %s and script provided "
+      "Blocking eval() %s from file %s and script provided "
       "%s",
+      (systemPrincipal ? "with System Principal" : "in parent process"),
       fileName.get(), NS_ConvertUTF16toUTF8(aScript).get());
 #else
-  MOZ_LOG(sCSMLog, LogLevel::Debug,
-          ("Blocking eval() with SystemPrincipal from file %s and script "
+  MOZ_LOG(sCSMLog, LogLevel::Warning,
+          ("Blocking eval() %s from file %s and script "
            "provided %s",
+           (systemPrincipal ? "with System Principal" : "in parent process"),
            fileName.get(), NS_ConvertUTF16toUTF8(aScript).get()));
 #endif
 

dom/security/nsContentSecurityManager.h

@@ -41,9 +41,9 @@ class nsContentSecurityManager : public nsIContentSecurityManager,
   static bool AllowInsecureRedirectToDataURI(nsIChannel* aNewChannel);
 
   static FilenameType FilenameToEvalType(const nsString& fileName);
-  static void AssertEvalNotUsingSystemPrincipal(JSContext* cx,
-                                                nsIPrincipal* aSubjectPrincipal,
-                                                const nsAString& aScript);
+  static void AssertEvalNotRestricted(JSContext* cx,
+                                      nsIPrincipal* aSubjectPrincipal,
+                                      const nsAString& aScript);
 
  private:
   static nsresult CheckChannel(nsIChannel* aChannel);

modules/libpref/init/StaticPrefList.yaml

@@ -5971,6 +5971,13 @@
   value: true
   mirror: always
 
+# Disabled by default so it doesn't affect Thunderbird/SeaMonkey, but
+# enabled in firefox.js
+- name: security.allow_eval_in_parent_process
+  type: bool
+  value: true
+  mirror: always
+
 # Whether strict file origin policy is in effect.
 - name: security.fileuri.strict_origin_policy
   type: RelaxedAtomicBool

toolkit/components/telemetry/Events.yaml

@@ -1593,11 +1593,12 @@ intl.ui.browserLanguage:
 
 security:
   evalUsage:
-    objects: ["systemContext"]
+    objects: ["systemContext", "parentProcess"]
     bug_numbers:
       - 1567623
     description: >
-      eval() (or an eval()-like method) was called while running in the System Principal Context.
+      eval() (or an eval()-like method) was called while running in the System Principal
+      context or the Parent Process.
       Expected values are:
         chromeuri - chrome:// file
         resourceuri - resource:// file