lockdown - breakout making globalThis properties non-writable (#12258)

* lockdown - breakout making globalThis properties non-writable into lockdown-more.js

* Update app/scripts/lockdown-more.js

Co-authored-by: David Walsh <davidwalsh83@gmail.com>

* Update app/scripts/lockdown-more.js

Co-authored-by: Erik Marks <25517051+rekmarks@users.noreply.github.com>

Co-authored-by: David Walsh <davidwalsh83@gmail.com>
Co-authored-by: Erik Marks <25517051+rekmarks@users.noreply.github.com>

.eslintrc.js

@@ -183,6 +183,7 @@ module.exports = {
         'nyc.config.js',
         'stylelint.config.js',
         'app/scripts/lockdown-run.js',
+        'app/scripts/lockdown-more.js',
         'development/**/*.js',
         'test/e2e/**/*.js',
         'test/lib/wait-until-called.js',
@@ -197,6 +198,7 @@ module.exports = {
     {
       files: [
         'app/scripts/lockdown-run.js',
+        'app/scripts/lockdown-more.js',
         'test/unit-global/protect-intrinsics.test.js',
       ],
       globals: {

app/background.html

@@ -8,6 +8,7 @@
     <script src="./sentry-install.js" type="text/javascript" charset="utf-8"></script>
     <script src="./lockdown-install.js" type="text/javascript" charset="utf-8"></script>
     <script src="./lockdown-run.js" type="text/javascript" charset="utf-8"></script>
+    <script src="./lockdown-more.js" type="text/javascript" charset="utf-8"></script>
     <script src="./runtime-cjs.js" type="text/javascript" charset="utf-8"></script>
     {{@each(it.jsBundles) => val}}
     <script src="{{val}}" type="text/javascript" charset="utf-8"></script>

app/home.html

@@ -14,6 +14,7 @@
     <script src="./sentry-install.js" type="text/javascript" charset="utf-8"></script>
     <script src="./lockdown-install.js" type="text/javascript" charset="utf-8"></script>
     <script src="./lockdown-run.js" type="text/javascript" charset="utf-8"></script>
+    <script src="./lockdown-more.js" type="text/javascript" charset="utf-8"></script>
     <script src="./runtime-cjs.js" type="text/javascript" charset="utf-8"></script>
     {{@each(it.jsBundles) => val}}
     <script src="{{val}}" type="text/javascript" charset="utf-8"></script>

app/manifest/_base.json

@@ -35,6 +35,7 @@
         "globalthis.js",
         "lockdown-install.js",
         "lockdown-run.js",
+        "lockdown-more.js",
         "contentscript.js"
       ],
       "run_at": "document_start",

app/notification.html

@@ -37,6 +37,7 @@
     <script src="./sentry-install.js" type="text/javascript" charset="utf-8"></script>
     <script src="./lockdown-install.js" type="text/javascript" charset="utf-8"></script>
     <script src="./lockdown-run.js" type="text/javascript" charset="utf-8"></script>
+    <script src="./lockdown-more.js" type="text/javascript" charset="utf-8"></script>
     <script src="./runtime-cjs.js" type="text/javascript" charset="utf-8"></script>
     {{@each(it.jsBundles) => val}}
     <script src="{{val}}" type="text/javascript" charset="utf-8"></script>

app/phishing.html

@@ -5,6 +5,7 @@
     <script src="./globalthis.js" type="text/javascript" charset="utf-8"></script>
     <script src="./lockdown-install.js" type="text/javascript" charset="utf-8"></script>
     <script src="./lockdown-run.js" type="text/javascript" charset="utf-8"></script>
+    <script src="./lockdown-more.js" type="text/javascript" charset="utf-8"></script>
     <script src="./phishing-detect.js"></script>
     <link rel="stylesheet" type="text/css" href="./index.css" title="ltr">
     <link rel="stylesheet" type="text/css" href="./index-rtl.css" title="rtl" disabled>

app/popup.html

@@ -14,6 +14,7 @@
     <script src="./sentry-install.js" type="text/javascript" charset="utf-8"></script>
     <script src="./lockdown-install.js" type="text/javascript" charset="utf-8"></script>
     <script src="./lockdown-run.js" type="text/javascript" charset="utf-8"></script>
+    <script src="./lockdown-more.js" type="text/javascript" charset="utf-8"></script>
     <script src="./runtime-cjs.js" type="text/javascript" charset="utf-8"></script>
     {{@each(it.jsBundles) => val}}
     <script src="{{val}}" type="text/javascript" charset="utf-8"></script>

app/scripts/lockdown-more.js

@@ -0,0 +1,91 @@
+// Make all "object" and "function" own properties of globalThis
+// non-configurable and non-writable, when possible.
+// We call a property that is non-configurable and non-writable,
+// "non-modifiable".
+try {
+  /**
+   * `lockdown` only hardens the properties enumerated by the
+   * universalPropertyNames constant specified in 'ses/src/whitelist'. This
+   * function makes all function and object properties on the start compartment
+   * global non-configurable and non-writable, unless they are already
+   * non-configurable.
+   *
+   * It is critical that this function runs at the right time during
+   * initialization, which should always be immediately after `lockdown` has been
+   * called. At the time of writing, the modifications this function makes to the
+   * runtime environment appear to be non-breaking, but that could change with
+   * the addition of dependencies, or the order of our scripts in our HTML files.
+   * Exercise caution.
+   *
+   * See inline comments for implementation details.
+   *
+   * We write this function in IIFE format to avoid polluting global scope.
+   */
+  (function protectIntrinsics() {
+    const namedIntrinsics = Reflect.ownKeys(new Compartment().globalThis);
+
+    // These named intrinsics are not automatically hardened by `lockdown`
+    const shouldHardenManually = new Set(['eval', 'Function']);
+
+    const globalProperties = new Set([
+      // universalPropertyNames is a constant added by lockdown to global scope
+      // at the time of writing, it is initialized in 'ses/src/whitelist'.
+      // These properties tend to be non-enumerable.
+      ...namedIntrinsics,
+
+      // TODO: Also include the named platform globals
+      // This grabs every enumerable property on globalThis.
+      // ...Object.keys(globalThis),
+    ]);
+
+    globalProperties.forEach((propertyName) => {
+      const descriptor = Reflect.getOwnPropertyDescriptor(
+        globalThis,
+        propertyName,
+      );
+
+      if (descriptor) {
+        if (descriptor.configurable) {
+          // If the property on globalThis is configurable, make it
+          // non-configurable. If it has no accessor properties, also make it
+          // non-writable.
+          if (hasAccessor(descriptor)) {
+            Object.defineProperty(globalThis, propertyName, {
+              configurable: false,
+            });
+          } else {
+            Object.defineProperty(globalThis, propertyName, {
+              configurable: false,
+              writable: false,
+            });
+          }
+        }
+
+        if (shouldHardenManually.has(propertyName)) {
+          harden(globalThis[propertyName]);
+        }
+      }
+    });
+
+    /**
+     * Checks whether the given propertyName descriptor has any accessors, i.e. the
+     * properties `get` or `set`.
+     *
+     * We want to make globals non-writable, and we can't set the `writable`
+     * property and accessor properties at the same time.
+     *
+     * @param {Object} descriptor - The propertyName descriptor to check.
+     * @returns {boolean} Whether the propertyName descriptor has any accessors.
+     */
+    function hasAccessor(descriptor) {
+      return 'set' in descriptor || 'get' in descriptor;
+    }
+  })();
+} catch (error) {
+  console.error('Protecting intrinsics failed:', error);
+  if (globalThis?.sentry.captureException) {
+    globalThis.sentry.captureException(
+      new Error(`Protecting intrinsics failed: ${error.message}`),
+    );
+  }
+}

app/scripts/lockdown-run.js

@@ -20,95 +20,3 @@ try {
     );
   }
 }
-
-// Make all "object" and "function" own properties of globalThis
-// non-configurable and non-writable, when possible.
-// We call the a property that is non-configurable and non-writable,
-// "non-modifiable".
-try {
-  /**
-   * `lockdown` only hardens the properties enumerated by the
-   * universalPropertyNames constant specified in 'ses/src/whitelist'. This
-   * function makes all function and object properties on the start compartment
-   * global non-configurable and non-writable, unless they are already
-   * non-configurable.
-   *
-   * It is critical that this function runs at the right time during
-   * initialization, which should always be immediately after `lockdown` has been
-   * called. At the time of writing, the modifications this function makes to the
-   * runtime environment appear to be non-breaking, but that could change with
-   * the addition of dependencies, or the order of our scripts in our HTML files.
-   * Exercise caution.
-   *
-   * See inline comments for implementation details.
-   *
-   * We write this function in IIFE format to avoid polluting global scope.
-   */
-  (function protectIntrinsics() {
-    const namedIntrinsics = Reflect.ownKeys(new Compartment().globalThis);
-
-    // These named intrinsics are not automatically hardened by `lockdown`
-    const shouldHardenManually = new Set(['eval', 'Function']);
-
-    const globalProperties = new Set([
-      // universalPropertyNames is a constant added by lockdown to global scope
-      // at the time of writing, it is initialized in 'ses/src/whitelist'.
-      // These properties tend to be non-enumerable.
-      ...namedIntrinsics,
-
-      // TODO: Also include the named platform globals
-      // This grabs every enumerable property on globalThis.
-      // ...Object.keys(globalThis),
-    ]);
-
-    globalProperties.forEach((propertyName) => {
-      const descriptor = Reflect.getOwnPropertyDescriptor(
-        globalThis,
-        propertyName,
-      );
-
-      if (descriptor) {
-        if (descriptor.configurable) {
-          // If the property on globalThis is configurable, make it
-          // non-configurable. If it has no accessor properties, also make it
-          // non-writable.
-          if (hasAccessor(descriptor)) {
-            Object.defineProperty(globalThis, propertyName, {
-              configurable: false,
-            });
-          } else {
-            Object.defineProperty(globalThis, propertyName, {
-              configurable: false,
-              writable: false,
-            });
-          }
-        }
-
-        if (shouldHardenManually.has(propertyName)) {
-          harden(globalThis[propertyName]);
-        }
-      }
-    });
-
-    /**
-     * Checks whether the given propertyName descriptor has any accessors, i.e. the
-     * properties `get` or `set`.
-     *
-     * We want to make globals non-writable, and we can't set the `writable`
-     * property and accessor properties at the same time.
-     *
-     * @param {Object} descriptor - The propertyName descriptor to check.
-     * @returns {boolean} Whether the propertyName descriptor has any accessors.
-     */
-    function hasAccessor(descriptor) {
-      return 'set' in descriptor || 'get' in descriptor;
-    }
-  })();
-} catch (error) {
-  console.error('Protecting intrinsics failed:', error);
-  if (globalThis.sentry && globalThis.sentry.captureException) {
-    globalThis.sentry.captureException(
-      new Error(`Protecting intrinsics failed: ${error.message}`),
-    );
-  }
-}

development/build/static.js

@@ -148,6 +148,12 @@ function getCopyTargets(shouldIncludeLockdown) {
         : EMPTY_JS_FILE,
       dest: `lockdown-run.js`,
     },
+    {
+      src: shouldIncludeLockdown
+        ? `./app/scripts/lockdown-more.js`
+        : EMPTY_JS_FILE,
+      dest: `lockdown-more.js`,
+    },
     {
       // eslint-disable-next-line node/no-extraneous-require
       src: require.resolve('@lavamoat/lavapack/src/runtime-cjs.js'),

test/unit-global/protect-intrinsics.test.js

@@ -1,5 +1,6 @@
 import 'ses/lockdown';
 import '../../app/scripts/lockdown-run';
+import '../../app/scripts/lockdown-more';
 import { strict as assert } from 'assert';
 
 // These are Agoric inventions, and we don't care about them.