fix(extension): stop leaking legacy instrument globals into the page

Extension/src/content/javascript-instrument-content-scope.ts

@@ -7,7 +7,9 @@ function getPageScriptAsString(
 ): string {
   // The JS Instrument Requests are setup and validated python side
   // including setting defaults for logSettings. See JSInstrumentation.py
-  const pageScriptString = `
+  // The IIFE keeps getInstrumentJS and jsInstrumentationSettings off the
+  // page's global scope.
+  const pageScriptString = `(function () {
 // Start of js-instruments.
 ${getInstrumentJS}
 // End of js-instruments.
@@ -19,7 +21,7 @@ const jsInstrumentationSettings = ${JSON.stringify(jsInstrumentationSettings)};
 // Start of anonymous function from javascript-instrument-page-scope.ts
 (${pageScript}(getInstrumentJS, jsInstrumentationSettings));
 // End.
-  `;
+})();`;
   return pageScriptString;
 }
 

Extension/src/lib/js-instruments.ts

@@ -51,19 +51,6 @@ export type SendMessagesToLogger = (
   messages: InstrumentMessage[],
 ) => void;
 
-declare global {
-  interface Object {
-    getPropertyDescriptor(
-      subject: any,
-      name: any,
-    ): PropertyDescriptor | undefined;
-  }
-
-  interface Object {
-    getPropertyNames(subject: any): string[];
-  }
-}
-
 interface CallContext {
   scriptUrl: string;
   scriptLine: string;
@@ -105,9 +92,11 @@ export function getInstrumentJS(
     set_prevented: "set(prevented)",
   };
 
-  // Rough implementations of Object.getPropertyDescriptor and Object.getPropertyNames
+  // Rough implementations of getPropertyDescriptor and getPropertyNames.
   // See http://wiki.ecmascript.org/doku.php?id=harmony:extended_object_api
-  Object.getPropertyDescriptor = function (subject, name) {
+  // Closure-local: assigning them onto the page-world `Object` is a
+  // detectable tell.
+  function getPropertyDescriptor(subject: any, name: any) {
     if (subject === undefined) {
       throw new Error("Can't get property descriptor for undefined");
     }
@@ -118,9 +107,9 @@ export function getInstrumentJS(
       proto = Object.getPrototypeOf(proto);
     }
     return pd;
-  };
+  }
 
-  Object.getPropertyNames = function (subject) {
+  function getPropertyNames(subject: any) {
     if (subject === undefined) {
       throw new Error("Can't get property names for undefined");
     }
@@ -132,7 +121,7 @@ export function getInstrumentJS(
     }
     // FIXME: remove duplicate property names from props
     return props;
-  };
+  }
 
   // debounce - from Underscore v1.6.0
   function debounce(
@@ -548,7 +537,7 @@ export function getInstrumentJS(
     }
 
     // Store original descriptor in closure
-    const propDesc = Object.getPropertyDescriptor(object, propertyName);
+    const propDesc = getPropertyDescriptor(object, propertyName);
 
     // Property descriptor must exist unless we are instrumenting a nonExisting property
     if (
@@ -741,7 +730,7 @@ export function getInstrumentJS(
     if (logSettings.propertiesToInstrument === null) {
       propertiesToInstrument = [];
     } else if (logSettings.propertiesToInstrument.length === 0) {
-      propertiesToInstrument = Object.getPropertyNames(object);
+      propertiesToInstrument = getPropertyNames(object);
     } else {
       propertiesToInstrument = logSettings.propertiesToInstrument;
     }

test/test_js_instrument.py

@@ -141,6 +141,57 @@ def test_instrument_object(self):
         )
 
 
+class TestJSInstrumentNoGlobalLeak(OpenWPMJSTest):
+    """The injected instrument must leave no page-visible globals (#1187).
+
+    Covers the getPropertyDescriptor / getPropertyNames helpers formerly
+    assigned onto ``Object``, plus the ``getInstrumentJS`` function and the
+    ``jsInstrumentationSettings`` const the injected script declared at top
+    level. Each is a config-independent tell (Krumnow, Jonker & Karsch,
+    arXiv:2205.08890 §4.1). The page encodes what it sees into the URL of an
+    instrumented ``fetch`` call.
+    """
+
+    TEST_PAGE = "instrument_no_global_leak.html"
+
+    METHOD_CALLS = {
+        (
+            "window.fetch",
+            "call",
+            '["https://gpd-false.example.com/gpn-false/gij-undefined/jis-undefined"]',
+        ),
+    }
+    GETS_AND_SETS: Set[Tuple[str, str, str]] = set()
+
+    def get_config(
+        self, data_dir: Optional[Path]
+    ) -> Tuple[ManagerParams, List[BrowserParams]]:
+        manager_params, browser_params = super().get_config(data_dir)
+        browser_params[0].prefs = {
+            "network.dns.localDomains": ("gpd-false.example.com,gpd-true.example.com")
+        }
+        browser_params[0].js_instrument_settings = [
+            {
+                "window": [
+                    "fetch",
+                ]
+            },
+        ]
+        return manager_params, browser_params
+
+    def test_no_global_leak(self):
+        db = self.visit("/js_instrument/%s" % self.TEST_PAGE)
+        top_url = f"{self.server.base}/js_instrument/{self.TEST_PAGE}"
+        self._check_calls(
+            db=db,
+            symbol_prefix="",
+            doc_url=top_url,
+            top_url=top_url,
+            expected_method_calls=self.METHOD_CALLS,
+            expected_gets_and_sets=self.GETS_AND_SETS,
+        )
+
+
 class TestJSInstrumentMockWindowProperty(OpenWPMJSTest):
     GETS_AND_SETS = {
         ("window.alreadyInstantiatedMockClassInstance", "get", "{}"),

test/test_pages/js_instrument/instrument_no_global_leak.html

@@ -0,0 +1,38 @@
+<!doctype html>
+<html>
+  <head>
+    <title>
+      Test page for JS Instrument - injected instrument must not leak page
+      globals (#1187)
+    </title>
+  </head>
+  <body>
+    <h3>
+      Test that the legacy JavaScript instrument leaves no page-visible
+      globals: no helpers on <code>Object</code>, no
+      <code>getInstrumentJS</code>, no <code>jsInstrumentationSettings</code>.
+    </h3>
+    <p>
+      The browser instruments <code>window.fetch</code> (configured python side)
+      so the instrument runs on this page. The observed state is encoded into
+      the URL of an instrumented <code>fetch</code> call so it lands in the
+      javascript table.
+    </p>
+    <script type="text/javascript">
+      const own = Object.getOwnPropertyNames(Object);
+      const gpd = own.indexOf("getPropertyDescriptor") !== -1;
+      const gpn = own.indexOf("getPropertyNames") !== -1;
+      const gij = typeof getInstrumentJS;
+      const jis = typeof jsInstrumentationSettings;
+      // localDomains maps these hosts so the request does not hit the network.
+      const probe =
+        "https://gpd-" + gpd + ".example.com/gpn-" + gpn +
+        "/gij-" + gij + "/jis-" + jis;
+      try {
+        window.fetch(probe);
+      } catch (e) {
+        console.log("fetch probe threw (ignored): ", e);
+      }
+    </script>
+  </body>
+</html>