Start compartment globalThis scuttling (browserify only) #360

weizman · 2022-08-08T14:31:51Z

This PR introduces @kumavis's idea for scuttling the globalThis object of the start compartment in LavaMoat.

Motivation

Since each package LavaMoat wraps has its own compartment and own set of intrinsics (including the app that is protected which is referred to as root package), this effectively means the real globalThis object is out of use and serves almost exclusively attackers only.

In other words, the main app has access to everything the globalThis object has to offer, but it goes through LavaMoat to do so which provides everything from the compartment - not the actual globalThis object.

However, if a package manages to access the real globalThis object which no code in the app really uses, it has access to sensitive APIs it shouldn't have.

Based on how this serves no purpose other then a malicious one, it would make sense to offer as a configurable feature in LavaMoat the option to disarm the globalThis object completely, by redefining all of its properties to not return their real values (aka scuttling).

Implementation

So when scuttling is turned on, any code that runs within the app and manages to access the globalThis object, will encounter a thrown error when trying to access anything it offers:

Issues

If you encounter an unexpected issue when running an app under scuttling mode, please let us know, that should not be the case.

If the app is trying to access a property that it should have access to, you can easily fix that by adding it to the lavamoatOpts.scuttleGlobalThisExceptions config property (see usage section below)

Many have reported issues with WebDriver manipulation of MM when this feature is on, please refer to my comment below for further info on this

Support

~~Currently this feature is only usable for browser based apps built with browserify~~
With #391 node/core/browserify support scuttling

Usage

By adding the scuttleGlobalThis=true config property, scuttling can be turned on (see here)

Optionally, you can also set scuttleGlobalThisException=[] property to skip certain props from being scuttled.

const lavamoatOpts = {

  // turn scuttling off
  scuttleGlobalThis: false, // true,

  // set scuttle exceptions
  scuttleGlobalThisException: [], // ['fetch', 'alert'],
}

WIP

Some later related PRs:

Scuttle start comp global this (fixes) #382
- some fixes requested by @kumavis after Start compartment globalThis scuttling (browserify only) #360 was merged
Fix for PR #382 [scuttling] #391
- some important fixes and updates, partially related to bugs introduces by Scuttle start comp global this (fixes) #382
Support re exp for scuttling avoid props #405
- support reg exps as excepted scuttling props rather than just hardcoded strings
Support configurable hooking into LavaMoat scuttling #462
- Support configurable hooking into LavaMoat scuttling

naugtur · 2022-08-16T07:02:21Z

packages/core/src/kernelCoreTemplate.js

@@ -72,6 +72,9 @@
    const rootPackageName = '$root$'
    const rootPackageCompartment = createRootPackageCompartment(globalRef)

+    // scuttle globalThis right after we used it to create the root package compartment
+    scuttleGlobalThis()


Feels like we'd want this to be configurable in the final version

https://github.com/LavaMoat/LavaMoat/pull/376/files
(ignore 'runtime.js` was added by mistake)

Hey I've tried disabling scuttling and it seems to work the first time. However, when running it with an automated software after that, the metamask extention does not seem to be able to load. Is there anything else that needs to be changed apart from:

await lavapack.buildRuntime({ scuttleGlobalThis: { enabled: false, scuttlerName: 'SCUTTLER', exceptions: scuttleGlobalThisExceptions, }, }); }

Is there a need to change/modify the exceptions param?

naugtur · 2022-08-16T07:04:27Z

packages/core/src/kernelCoreTemplate.js

+
+      for (const prop of props) {
+        if (!avoids.includes(prop)) {
+          Object.defineProperty(window, prop, {value: undefined})


How about a getter that throws with helpful information?

also, for scuttling to work, lockdown umd must access the new scoped version of Error, instead of directly via globalThis, so had to fix that too 7601883
is this how we update lockdown umd? should it be updated also elsewhere?

naugtur · 2022-09-19T10:12:16Z

packages/core/lib/lockdown.umd.js

@@ -3027,7 +3027,7 @@ const tagError = (err, optErrorName = err.name) => {
 */
 const makeError = (
 optDetails = redactedDetails`Assert failed`,
-ErrorConstructor = globalThis.Error,
+ErrorConstructor = Error,


You can't modify this. It's a vendored SES build

yea i was asking about it here

what are my options then? can we change that at SES? any reason for having to use globalThis.Error instead of just Error?

as agreed with @naugtur,

undo any changes made to this file 5ebd45b and avoid scuttling Error 785b4a7

naugtur · 2022-09-19T10:24:02Z

packages/core/src/kernelCoreTemplate.js

+      }
+      if (globalThis?.EventTarget?.prototype) {
+        props = [...props, ...Object.getOwnPropertyNames(EventTarget.prototype)]
+      }


I'm not sure if the browser optimizes this. Probably not. It might be more efficient to collect the properties inside ifs into some variables and then avoiding multiple spreads on props.

naugtur · 2022-09-19T10:25:15Z

packages/core/src/kernelCoreTemplate.js

@@ -82,6 +90,43 @@
    Object.freeze(kernel)
    return kernel

+    function scuttleGlobalThis(extraAvoids = new Array()) {
+      let props = [...Object.getOwnPropertyNames(globalThis)]


Why copy the result of getOwnPropertyNames? it's not like something else is using this reference

henzycuong1 · 2023-09-03T04:57:11Z

mark: ==>>

hi, my metamask has been corrupted after change that. how it work ?

cdoky · 2023-09-03T05:37:04Z

@henzycuong1 What happend with you

henzycuong1 · 2023-09-03T05:41:04Z

What happend with you

sorry, it worked. But i have to reinstall my metamask, that caused the extension's id be changed. Anyway i can fix it on old metamask extension or keep the extension's id is "nkbihfbeogaeaoehlefnkodbefgpgknn"

jhxiyu · 2024-03-03T16:00:51Z

sentry-install.js:1 Property eval will not be hardened because it is scuttled by LavaMoat protection. Visit #360 to learn more.
e. @ sentry-install.js:1
sentry-install.js:1 Translator - Unable to find value of key "buyAndSell" for locale "zh_CN"
e. @ sentry-install.js:1
popup.html#confirmation:1 [Intervention] Slow network is detected. See https://www.chromestatus.com/feature/5636954674692096 for more details. Fallback font will be used while loading: chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/fonts/Euclid/EuclidCircularB-Regular-WebXL.ttf
8Framework Event Listeners API Errors:
devtoolsFrameworkEventListeners call produced error: Error: LavaMoat - property "self" of globalThis is inaccessible under scuttling mode. To learn more visit #360.
fetcher call produced error: Error: LavaMoat - property "Node" of globalThis is inaccessible under scuttling mode. To learn more visit #360
e. @ sentry-install.js:1
sentry-install.js:1 Translator - Unable to find value of key "buyAndSell" for locale "zh_CN"
e. @ sentry-install.js:1
popup.html#confirmation:1 [Intervention] Slow network is detected. See https://www.chromestatus.com/feature/5636954674692096 for more details. Fallback font will be used while loading: chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/fonts/Euclid/EuclidCircularB-Regular-WebXL.ttf

iniyavans · 2024-03-23T11:19:24Z

It works only after changing the value of 'scuttleGlobalThis' to enable status as true from false.

## **Description** When loading a prod mv3 build, we observed that a majority of the time, the offscreen document would not be created. In those case, we saw the following error in the service worker console: ``` runtime-lavamoat.js:13190 Uncaught Error: LavaMoat - property "chrome" of globalThis is inaccessible under scuttling mode. To learn more visit LavaMoat/LavaMoat#360. at Object.get (runtime-lavamoat.js:13190:11) at app-init.js:1:1766 get @ runtime-lavamoat.js:13190 (anonymous) @ app-init.js:1 ``` Which comes from this code in `app-init.js`: ``` async function createOffscreen() { if (await chrome.offscreen.hasDocument()) { return; } await chrome.offscreen.createDocument({ url: './offscreen.html', reasons: ['IFRAME_SCRIPTING'], justification: 'Used for Hardware Wallet and Snaps scripts to communicate with the extension.', }); console.debug('Offscreen iframe loaded'); } ``` The solution is to cache a version of chrome before it has been scuttled. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/24103?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. `yarn dist:mv3` 2. Install at chrome://extensions 3. A second or so after the service worker is created, the offscreen document should be created. 4. Reload the extension 10 times, the offscreen document should be created every time ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

… on three confirmations concurrently` (#25675)  ## **Description** The queue UI test fails has a couple of flaky points. Sometimes we see the console errors for network unresponsive -- when this happens the test fails. When we don't see the console errors for network unresponsive the test passes. The problem is that in the tests we were ignoring the PollingBlock errors, but more errors appear when the network is down, making the test fail when they appear in the console. ![Screenshot from 2024-07-05 14-45-37](https://github.com/MetaMask/metamask-extension/assets/54408225/4f105322-3bd0-4908-a107-8984fce1915d) https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/90602/workflows/e41e1e65-d445-4b60-9602-89a261cc5654/jobs/3362082/parallel-runs/4?filterBy=ALL There are more sources of flakiness, that can also cause this test to fail. Those are described here and fixed in this same PR: - Performing a subsequent action when the popup is closed, causes the `NoSuchWindowError: no such window: target window already closed` error. We need to switch back to an existing window, before proceeding with test actions - Triggering a tx/signature with an unresponsive network (when we kill ganache) takes a significant amount of time to open the popup. Now we wait for the expected number of windows before switching to the popup and we add a big timeout for this case (it's a max value and won't delay the test if it's not needed, like in FF). There's an issue open to fix this on the app level in [this ticket](#25690). - Asserting values from elements found by their css/testid can introduce flakiness as the elements could be rendered without having the expected text yet loaded. Now the approach for these validations has been fixed by looking for the exact selector which includes the text - A lavamoat error appeared once, but this seem unrelated to this specific test and just saw it once so it's left outside the scope of this PR `error caught in retry(): JavascriptError: javascript error: LavaMoat - property "name" of globalThis is inaccessible under scuttling mode. To learn more visit https://github.com/LavaMoat/LavaMoat/pull/360.` - Failure 2: - https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/90432/workflows/81444bef-1d2c-4da0-bf60-1c0c51a5b530/jobs/3353522/parallel-runs/17 - Logs: ``` [driver] Called 'switchToWindowWithTitle' with arguments ["MetaMask Dialog",["8C425F6EC00FF98E5C0A643C0CDF0DD6","0BF6602327E4E63F0FAE641DF2BB71E0","45802EBF75EF61BE99D7F235354A0FC6","109C90EB942C664573D051FD7952C0CE","4199B96477AFE13BB203FFC80C399B9F"]] [driver] Called 'findClickableElement' with arguments ["[data-testid=\"confirmation-submit-button\"]"] [driver] Called 'clickElement' with arguments ["[data-testid=\"confirmation-submit-button\"]"] [driver] Called 'openNewPage' with arguments ["http://127.0.0.1:8082"] Failed to handle request: socket hang up Failure on testcase: 'Request-queue UI changes handles three confirmations on three confirmations concurrently @no-mmi', for more information see the artifacts tab in CI NoSuchWindowError: no such window: target window already closed from unknown error: web view not found ``` - Failure 3: - https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/90534/workflows/87faa6e4-aa65-4273-8d7e-d369782cfe1d/jobs/3358659/parallel-runs/2 - Logs from error 2: ``` JsonRpcEngine: Response has no error or result for request: { "jsonrpc": "2.0", "id": "8871ca57-2969-45f2-8838-115e34bfa417", "method": "eth_blockNumber", "params": [], "origin": "metamask", "networkClientId": "networkConfigurationId", "tabId": 1870836432 } at m._fetchLatestBlock (chrome-extension://aohcjlklkjjpdgebibknknkinbdpdmoe/common-1.js:1:357185) at async m._updateLatestBlock (chrome-extension://aohcjlklkjjpdgebibknknkinbdpdmoe/common-1.js:1:356869) at async m._updateAndQueue (chrome-extension://aohcjlklkjjpdgebibknknkinbdpdmoe/common-1.js:1:357352) at m._updateAndQueue (chrome-extension://aohcjlklkjjpdgebibknknkinbdpdmoe/common-1.js:1:357401) ----------End of Chrome error---------- ---This error is on the ignore list---- [driver] Called 'findElement' with arguments ["[data-testid=\"network-display\"], [data-testid=\"signature-request-network-display\"]"] [driver] Called 'findElement' with arguments [".confirm-page-container-summary__origin bdi, .request-signature__origin .chip__label"] [driver] Called 'executeScript' with arguments [null] Failure on testcase: 'Request-queue UI changes should gracefully handle network connectivity failure for confirmations @no-mmi', for more information see the artifacts tab in CI ``` - Failure 4: hasn't appeared but since it could potentially appear in the future, has also been fixed https://github.com/MetaMask/metamask-extension/assets/54408225/4ba1effc-61a4-4fe5-b4a9-c2785856eba3 [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25675?quickstart=1) ## **Related issues** Fixes: #25674 ## **Manual testing steps** 1. Check ci ## **Screenshots/Recordings** ![Screenshot from 2024-07-04 09-03-08](https://github.com/MetaMask/metamask-extension/assets/54408225/8acaf835-7b25-4fb6-b0fc-fa01f026d627) ![Screenshot from 2024-07-05 10-03-57](https://github.com/MetaMask/metamask-extension/assets/54408225/22530cc1-376b-4075-9871-43fff63bbe10) ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

phakel21 · 2024-07-05T18:11:36Z

I have same problem and when change

await lavapack.buildRuntime({ scuttleGlobalThis: { enabled: applyLavaMoat && shouldScuttle, scuttlerName: 'SCUTTLER', exceptions: scuttleGlobalThisExceptions, }, });

to

await lavapack.buildRuntime({ scuttleGlobalThis: { enabled: false, scuttlerName: 'SCUTTLER', exceptions: scuttleGlobalThisExceptions, }, });

i get MetaMask Offscreen Page where can't find any element by selenium webdriver

x5engine · 2024-07-27T14:20:35Z

Hello guys, I come from metamask console log :)

SantiiRepair · 2024-08-09T18:08:50Z

mark: ==>>

hi, my metamask has been corrupted after change that. how it work ?

Same here

SantiiRepair · 2024-08-09T18:13:43Z

Changing it to false breaks the extension, when you close the browser and open it and then open the extension it doesn't work anymore, even chrome deletes it.

PS: I edited the runtime-lavamoat.js file in the extension directory inside chrome, in case you were wondering.

f-javier-aguerri · 2025-01-03T08:42:26Z

Hi, using yarn download-builds:test breaks the app. When opened, the metamask extension shows up as a black pop-up.

agupagu · 2025-02-06T01:38:27Z

Hi I'm trying to use Browser-Use to automate metamask and have used yarn start to build a local build of metamask with lavamoat disabled. However, I am still getting these errors. Anyone can help?

agupagu · 2025-02-11T08:11:53Z

Hi anybody can help with the above? Would love some advice

weizman added 14 commits August 8, 2022 17:31

poc of globalThis start compartment scutteling

e39b592

dont scuttle reflect so lm works

b02c602

refactor and better support reflect access

ec79eab

add comma

5dc5372

move scuttling before deps load, also skeep document scuttling for now

0a639d3

add APIs to avoid scuttling so mm works

05749fa

no need to use native reflect anymore

90d944a

Update makeGeneralUtils.js

93cb14a

Update makeGeneralUtils.js

a1f2d5d

Update makeGeneralUtils.js

0821a58

remove \n

72c8181

improve how scuttling happens

fba95e6

Update kernelCoreTemplate.js

3e2831d

remove new lines

1339d64

naugtur reviewed Aug 16, 2022

View reviewed changes

weizman added 7 commits September 7, 2022 11:30

Merge branch 'main' into scuttle-start-comp-global-this

009c945

Scuttle start comp global this - add config support (#376)

4eb5e2b

Merge branch 'main' into scuttle-start-comp-global-this

8c7d639

globalThis instead of window

340344d

revert runtime

551de05

throw error when accessing props under scuttle mode

fd49fac

fix lockdown umd to work properly with scuttling

7601883

weizman changed the title ~~[DRAFT] poc of globalThis start compartment scutteling~~ Start compartment globalThis scuttling (browserify only) Sep 18, 2022

weizman marked this pull request as ready for review September 18, 2022 11:48

weizman requested a review from naugtur September 18, 2022 12:38

support extra avoids array as scuttling config

220db7e

naugtur reviewed Sep 19, 2022

View reviewed changes

github-actions bot mentioned this pull request Aug 17, 2023

chore: release main #665

Merged

th0masi mentioned this pull request Oct 22, 2023

Endless loading of the custom build of MetaMask Extension MetaMask/metamask-extension#21484

Open

weizman mentioned this pull request Jan 29, 2024

refactor(core): extract scuttling from the kernel #758

Merged

3 tasks

mo-bay mentioned this pull request Mar 26, 2024

[Bug]: Unable to Access MetaMask Wallet after Network Issues MetaMask/metamask-extension#23736

Closed

goushijie mentioned this pull request Sep 16, 2024

New tab throws an exception g1879/DrissionPage#380

Open

jerajx mentioned this pull request Oct 9, 2024

javascript error: LavaMoat - property "open" of globalThis is inaccessible under scuttling mode timelyart/Kairos#122

Closed

itaven mentioned this pull request Nov 5, 2024

fix: mm connect flow lidofinance/wallets-testing-modules#198

Merged

alshowash mentioned this pull request Jan 10, 2025

infinite loading MetaMask/metamask-extension#29645

Open

9 tasks

faithsloan mentioned this pull request Jan 16, 2025

[Bug]: Completely unusable Metamask infinite load MetaMask/metamask-extension#22533

Closed

leotm mentioned this pull request Jan 28, 2025

chore(core): run scenarios in ava context #996

Merged

sandywall26 mentioned this pull request Feb 3, 2025

[Bug]: If the transaction is used very slowly loading, and ask to start again, and extension appears Code Error MetaMask/metamask-extension#30045

Open

agupagu mentioned this pull request Feb 6, 2025

Automation and sentry-js error #1536

Open

grayguest mentioned this pull request Feb 21, 2025

[Question] What is the best way to scroll page to the bottom? microsoft/playwright#4302

Closed

phuc132d2 mentioned this pull request May 12, 2025

erro #1653

Closed

Masbal381 mentioned this pull request Jun 3, 2025

MetaMask stuck on loading screen in Firefox, no password prompt shown MetaMask/metamask-extension#33445

Open

9 tasks

Masbal381 mentioned this pull request Jun 11, 2025

[Bug]: MetaMask is stuck in an infinite loading state in Firefox MetaMask/metamask-extension#33586

Open

Start compartment globalThis scuttling (browserify only) #360

Start compartment globalThis scuttling (browserify only) #360

Conversation

weizman commented Aug 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Implementation

Issues

Support

Usage

WIP

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

henzycuong1 commented Sep 3, 2023

Uh oh!

cdoky commented Sep 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

henzycuong1 commented Sep 3, 2023

Uh oh!

jhxiyu commented Mar 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iniyavans commented Mar 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

phakel21 commented Jul 5, 2024

Uh oh!

x5engine commented Jul 27, 2024

Uh oh!

SantiiRepair commented Aug 9, 2024

Uh oh!

SantiiRepair commented Aug 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

f-javier-aguerri commented Jan 3, 2025

Uh oh!

agupagu commented Feb 6, 2025

Uh oh!

agupagu commented Feb 11, 2025

Uh oh!

Uh oh!

weizman commented Aug 8, 2022 •

edited

Loading

cdoky commented Sep 3, 2023 •

edited

Loading

jhxiyu commented Mar 3, 2024 •

edited

Loading

iniyavans commented Mar 23, 2024 •

edited

Loading

SantiiRepair commented Aug 9, 2024 •

edited

Loading