Freeze Promise global on boot (#7309)

* freeze background and UI Promise globals on boot

* add new tests

* remove tape

.circleci/config.yml

@@ -35,6 +35,9 @@ workflows:
       - test-unit:
           requires:
             - prep-deps
+      - test-unit-global:
+          requires:
+            - prep-deps
       - test-mozilla-lint:
           requires:
             - prep-deps
@@ -51,6 +54,7 @@ workflows:
           requires:
             - test-lint
             - test-unit
+            - test-unit-global
             - test-mozilla-lint
             - test-e2e-chrome
             - test-e2e-firefox
@@ -310,6 +314,16 @@ jobs:
           paths:
             - .nyc_output
             - coverage
+  test-unit-global:
+    docker:
+      - image: circleci/node:10.16-browsers
+    steps:
+      - checkout
+      - attach_workspace:
+          at: .
+      - run:
+          name: test:unit:global
+          command: yarn test:unit:global
   test-mozilla-lint:
     docker:
       - image: circleci/node:10.16-browsers

app/scripts/background.js

@@ -2,7 +2,9 @@
  * @file The entry point for the web extension singleton process.
  */
 
-// this needs to run before anything else
+
+// these need to run before anything else
+require('./lib/freezeGlobals')
 require('./lib/setupFetchDebugging')()
 
 // polyfills

app/scripts/lib/freezeGlobals.js

@@ -0,0 +1,41 @@
+
+/**
+ * Freezes the Promise global and prevents its reassignment.
+ */
+const deepFreeze = require('deep-freeze-strict')
+
+if (
+  process.env.IN_TEST !== 'true' &&
+  process.env.METAMASK_ENV !== 'test'
+) {
+  freeze(global, 'Promise')
+}
+
+/**
+ * Makes a key:value pair on a target object immutable, with limitations.
+ * The key cannot be reassigned or deleted, and the value is recursively frozen
+ * using Object.freeze.
+ *
+ * Because of JavaScript language limitations, this is does not mean that the
+ * value is completely immutable. It is, however, better than nothing.
+ *
+ * @param {Object} target - The target object to freeze a property on.
+ * @param {String} key - The key to freeze.
+ * @param {any} [value] - The value to freeze, if different from the existing value on the target.
+ * @param {boolean} [enumerable=true] - If given a value, whether the property is enumerable.
+ */
+function freeze (target, key, value, enumerable = true) {
+
+  const opts = {
+    configurable: false, writable: false,
+  }
+
+  if (value !== undefined) {
+    opts.value = deepFreeze(value)
+    opts.enumerable = enumerable
+  } else {
+    target[key] = deepFreeze(target[key])
+  }
+
+  Object.defineProperty(target, key, opts)
+}

app/scripts/ui.js

@@ -1,3 +1,7 @@
+
+// this must run before anything else
+require('./lib/freezeGlobals')
+
 // polyfills
 import 'abortcontroller-polyfill/dist/polyfill-patch-fetch'
 

package.json

@@ -15,6 +15,7 @@
     "watch:test:unit": "nodemon --exec \"yarn test:unit\" ./test ./app ./ui",
     "sendwithprivatedapp": "static-server test/e2e/send-eth-with-private-key-test --port 8080",
     "test:unit": "cross-env METAMASK_ENV=test mocha --exit --require test/setup.js --recursive \"test/unit/**/*.js\" \"ui/app/**/*.test.js\"",
+    "test:unit:global": "mocha test/unit-global/*",
     "test:single": "cross-env METAMASK_ENV=test mocha --require test/helper.js",
     "test:integration": "yarn test:integration:build && yarn test:flat",
     "test:integration:build": "gulp build:scss",
@@ -80,6 +81,7 @@
     "debounce": "1.1.0",
     "debounce-stream": "^2.0.0",
     "deep-extend": "^0.5.1",
+    "deep-freeze-strict": "1.1.1",
     "detect-node": "^2.0.3",
     "detectrtc": "^1.3.6",
     "dnode": "^1.2.2",
@@ -201,7 +203,6 @@
     "coveralls": "^3.0.0",
     "cross-env": "^5.1.4",
     "css-loader": "^2.1.1",
-    "deep-freeze-strict": "^1.1.1",
     "del": "^3.0.0",
     "deps-dump": "^1.1.0",
     "envify": "^4.0.0",
@@ -283,7 +284,6 @@
     "style-loader": "^0.21.0",
     "stylelint": "^9.10.1",
     "stylelint-config-standard": "^18.2.0",
-    "tape": "^4.5.1",
     "testem": "^2.16.0",
     "through2": "^2.0.3",
     "vinyl-buffer": "^1.0.1",

test/unit-global/frozenPromise.js

@@ -0,0 +1,55 @@
+
+/* eslint-disable no-native-reassign */
+
+// this is what we're testing
+require('../../app/scripts/lib/freezeGlobals')
+
+const assert = require('assert')
+
+describe('Promise global is immutable', () => {
+
+  it('throws when reassinging promise (syntax 1)', () => {
+    try {
+      Promise = {}
+      assert.fail('did not throw error')
+    } catch (err) {
+      assert.ok(err, 'did throw error')
+    }
+  })
+
+  it('throws when reassinging promise (syntax 2)', () => {
+    try {
+      global.Promise = {}
+      assert.fail('did not throw error')
+    } catch (err) {
+      assert.ok(err, 'did throw error')
+    }
+  })
+
+  it('throws when mutating existing Promise property', () => {
+    try {
+      Promise.all = () => {}
+      assert.fail('did not throw error')
+    } catch (err) {
+      assert.ok(err, 'did throw error')
+    }
+  })
+
+  it('throws when adding new Promise property', () => {
+    try {
+      Promise.foo = 'bar'
+      assert.fail('did not throw error')
+    } catch (err) {
+      assert.ok(err, 'did throw error')
+    }
+  })
+
+  it('throws when deleting Promise from global', () => {
+    try {
+      delete global.Promise
+      assert.fail('did not throw error')
+    } catch (err) {
+      assert.ok(err, 'did throw error')
+    }
+  })
+})

yarn.lock

@@ -8018,7 +8018,7 @@ deep-extend@^0.6.0, deep-extend@~0.6.0:
   resolved "https://registry.yarnpkg.com/deep-extend/-/deep-extend-0.6.0.tgz#c4fa7c95404a17a9c3e8ca7e1537312b736330ac"
   integrity sha512-LOHxIOaPYdHlJRtCQfDIVZtfw/ufM8+rVj649RIHzcm/vGwQRXFt6OPqIFWsm2XEMrNIEtWR64sY1LEKD2vAOA==
 
-deep-freeze-strict@^1.1.1:
+deep-freeze-strict@1.1.1:
   version "1.1.1"
   resolved "https://registry.yarnpkg.com/deep-freeze-strict/-/deep-freeze-strict-1.1.1.tgz#77d0583ca24a69be4bbd9ac2fae415d55523e5b0"
   integrity sha1-d9BYPKJKab5LvZrC+uQV1VUj5bA=
@@ -25510,7 +25510,7 @@ tapable@^1.0.0, tapable@^1.1.0:
   resolved "https://registry.yarnpkg.com/tapable/-/tapable-1.1.3.tgz#a1fccc06b58db61fd7a45da2da44f5f3a3e67ba2"
   integrity sha512-4WK/bYZmj8xLr+HUCODHGF1ZFzsYffasLUgEiMBY4fgtltdO6B4WJtlSbPaDTLpYTcGVwM2qLnFTICEcNxs3kA==
 
-tape@^4.5.1, tape@^4.6.3, tape@^4.8.0:
+tape@^4.6.3, tape@^4.8.0:
   version "4.8.0"
   resolved "https://registry.yarnpkg.com/tape/-/tape-4.8.0.tgz#f6a9fec41cc50a1de50fa33603ab580991f6068e"
   integrity sha512-TWILfEnvO7I8mFe35d98F6T5fbLaEtbFTG/lxWvid8qDfFTxt19EBijWmB4j3+Hoh5TfHE2faWs73ua+EphuBA==