Skip to content

Commit

Permalink
feat: SES lockdown v0.18.8 (iOS JSC) (#6586)
Browse files Browse the repository at this point in the history
## **Description**

Problem being solved: prototype pollution/poisoning

SES lockdown (shim v0.18.8) on iOS JSC, baked early into RN core before
RN initialisation for the simplest minimal solution
as opposed to previous approach of shim'ing at the beginning of our
entry file requiring further complex lib patches

with SES lockdown on Android Hermes (introduced earlier in our [RN v0.71.6 upgrade](#6220))
being followed up separately
currently bundling successfully, but runtime not yet functional

_SES lockdown on Android JSC was also passing smoke tests after some work prior to Hermes_
_so a backup engine worth keeping on ice being followed up separately_ 

_Previous patches no longer required:
[`eth-keyring-controller`](https://github.com/MetaMask/metamask-mobile/pull/3794/files#diff-19aae36749eec9908c74557591fade5cc596e9a40422d88594c0fba456870389),
`ethjs-contract` (one not [two](https://github.com/MetaMask/metamask-mobile/pull/3794/files#diff-1970015453fca9583682c37a44695a3d26645329e586bb70ff6f05b74f936802)), [`web3-core-methods`](LavaMoat/docs#8), [`metro-react-native-babel-preset`](LavaMoat/docs#4), Sentry [config](LavaMoat/docs#6) (see previous PR: #3794

_Nb: `@babel/plugin-transform-regenerator` removed from
`metro-react-native-babel-preset` since initial investigation_
_Nb: `@babel/plugin-transform-runtime` config opt `regenerator: true`
previously caused iOS animated node assertion failures_
_Nb: default `@babel/plugin-transform-runtime` via
`metro-react-native-babel-preset` causes additional 4 SES warnings_

<details>
<summary>Nb: Current behaviour (not SES)</summary>

```
main, jsc
- import wallet via SRP
  - tap form field: disables cmd+D (app must be closed, no Metro restart)

main, v8
- (fresh) import wallet via SRP
  - if cmd+v paste ok, bot Import btn tappable when filled, but spinner hang (no error/warn), ~20s Metro dc
  - restart, enter pw, tap Unlock btn, spinner hang, still ~20s Metro dc
  - tap Reset Wallet, hang
- import (continued)
  - partial responsiveness
    - tap form field: dev menu disabled (cmd+d, app must be closed)
    - unresponsive: tap Back, tap Show, cannot tap Import, cmd+v disabled
    - responsive: cycle/input fields
    - still ~20s Metro dc
```

</details>

<details>
<summary>Previous SES warnings when locking down at entry file (not RN
InitializeCore)</summary>

https://www.diffchecker.com/fjj1iObp

```console
# JSC (26)
Removing intrinsics.Object.setPrototypeOf.default
Removing intrinsics.Object.setPrototypeOf.__esModule
Removing intrinsics.Object.assign.default
Removing intrinsics.Object.assign.__esModule
Removing intrinsics.Reflect.construct.default
Removing intrinsics.Reflect.construct.__esModule
Removing intrinsics.Reflect.decorate
Removing intrinsics.Reflect.metadata
Removing intrinsics.Reflect.defineMetadata
Removing intrinsics.Reflect.hasMetadata
Removing intrinsics.Reflect.hasOwnMetadata
Removing intrinsics.Reflect.getMetadata
Removing intrinsics.Reflect.getOwnMetadata
Removing intrinsics.Reflect.getMetadataKeys
Removing intrinsics.Reflect.getOwnMetadataKeys
Removing intrinsics.Reflect.deleteMetadata
Removing intrinsics.%ArrayPrototype%.toReversed
Removing intrinsics.%ArrayPrototype%.toSorted
Removing intrinsics.%ArrayPrototype%.toSpliced
Removing intrinsics.%ArrayPrototype%.with
Removing intrinsics.%ArrayPrototype%.@@unscopables.toReversed
Removing intrinsics.%ArrayPrototype%.@@unscopables.toSorted
Removing intrinsics.%ArrayPrototype%.@@unscopables.toSpliced
Removing intrinsics.%TypedArrayPrototype%.toReversed
Removing intrinsics.%TypedArrayPrototype%.toSorted
Removing intrinsics.%TypedArrayPrototype%.with
```

```console
# V8 (33)
Removing intrinsics.Object.assign.default
Removing intrinsics.Object.assign.__esModule
Removing intrinsics.Object.setPrototypeOf.default
Removing intrinsics.Object.setPrototypeOf.__esModule
Removing intrinsics.JSON.rawJSON
Removing intrinsics.JSON.isRawJSON
Removing intrinsics.Reflect.construct.default
Removing intrinsics.Reflect.construct.__esModule
Removing intrinsics.Reflect.decorate
Removing intrinsics.Reflect.metadata
Removing intrinsics.Reflect.defineMetadata
Removing intrinsics.Reflect.hasMetadata
Removing intrinsics.Reflect.hasOwnMetadata
Removing intrinsics.Reflect.getMetadata
Removing intrinsics.Reflect.getOwnMetadata
Removing intrinsics.Reflect.getMetadataKeys
Removing intrinsics.Reflect.getOwnMetadataKeys
Removing intrinsics.Reflect.deleteMetadata
Removing intrinsics.%ArrayPrototype%.toReversed
Removing intrinsics.%ArrayPrototype%.toSorted
Removing intrinsics.%ArrayPrototype%.toSpliced
Removing intrinsics.%ArrayPrototype%.with
Removing intrinsics.%ArrayPrototype%.@@unscopables.toReversed
Removing intrinsics.%ArrayPrototype%.@@unscopables.toSorted
Removing intrinsics.%ArrayPrototype%.@@unscopables.toSpliced
Removing intrinsics.%ArrayBufferPrototype%.transferToFixedLength
Removing intrinsics.%ArrayBufferPrototype%.detached
Removing intrinsics.%StringPrototype%.isWellFormed
Removing intrinsics.%StringPrototype%.toWellFormed
Removing intrinsics.%RegExpPrototype%.unicodeSets
Removing intrinsics.%TypedArrayPrototype%.toReversed
Removing intrinsics.%TypedArrayPrototype%.toSorted
Removing intrinsics.%TypedArrayPrototype%.with
```

</details>

<details>
<summary>Notes on patch creation</summary>

- `--exclude 'nothing'` to include `package.json` changes, then trim
patch
- `react-native` requires trimming majority of patch after initial diffs
- upon failure on symlinks, `git clean -fdx` and re-create

</details>

## **Related issues**

Fixes
- LavaMoat/docs#1 various issues
  - #3794 previous pr
- LavaMoat/docs#12 various issues

Worthy read for everyone on adding/upgrading libraries
- endojs/endo#1855

## **Manual testing steps**

App functions normally

## **Screenshots/Recordings**

### **Before**

Previously failing iOS (JSC) E2E tests have now been fixed

- LavaMoat/docs#9
  - https://github.com/MetaMask/metamask-mobile/assets/1881059/3cac630f-6ec8-4975-a273-8a115a4e8fe9
  - https://github.com/MetaMask/metamask-mobile/assets/1881059/67221672-d1a3-4346-a783-5f93b53dbbe9
  - https://github.com/MetaMask/metamask-mobile/assets/1881059/baa6e512-e307-4f0f-ae2f-77bfb4a29baf
  - https://github.com/MetaMask/metamask-mobile/assets/1881059/c21ffb0b-c9df-4223-ba73-5383dc8627f5

And more screenshots in related issues linked above

### **After**

App functions normally

---------

Co-authored-by: legobeat <[email protected]>
  • Loading branch information
leotm and legobeat authored Nov 10, 2023
1 parent 40f25a8 commit 6edfe34
Show file tree
Hide file tree
Showing 7 changed files with 11,291 additions and 1 deletion.
15 changes: 15 additions & 0 deletions app/store/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,21 @@ const createStoreAndPersistor = async () => {
* Initialize services after persist is completed
*/
const onPersistComplete = () => {
/**
* EngineService.initalizeEngine(store) with SES/lockdown:
* Requires ethjs nested patches (lib->src)
* - ethjs/ethjs-query
* - ethjs/ethjs-contract
* Otherwise causing the following errors:
* - TypeError: Cannot assign to read only property 'constructor' of object '[object Object]'
* - Error: Requiring module "node_modules/ethjs/node_modules/ethjs-query/lib/index.js", which threw an exception: TypeError:
* - V8: Cannot assign to read only property 'constructor' of object '[object Object]'
* - JSC: Attempted to assign to readonly property
* - node_modules/babel-runtime/node_modules/regenerator-runtime/runtime.js
* - V8: TypeError: _$$_REQUIRE(...) is not a constructor
* - TypeError: undefined is not an object (evaluating 'TokenListController.tokenList')
* - V8: SES_UNHANDLED_REJECTION
*/
EngineService.initalizeEngine(store);
Authentication.init(store);
LockManagerService.init(store);
Expand Down
1 change: 1 addition & 0 deletions babel.config.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// eslint-disable-next-line import/no-commonjs
module.exports = {
ignore: [/ses\.cjs/],
presets: ['module:metro-react-native-babel-preset'],
plugins: [
'transform-inline-environment-variables',
Expand Down
9 changes: 8 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
// Importing SES (0.18.7+) here then calling lockdown causes:
// https://github.com/LavaMoat/docs/issues/24

// Importing the SES (0.18.7) lockdown shim here then calling lockdown causes:
// https://github.com/LavaMoat/docs/issues/27

import './shim.js';

// Needed to polyfill random number generation.
import 'react-native-get-random-values';
import '@walletconnect/react-native-compat';
import './shim.js';

import 'react-native-gesture-handler';
import 'react-native-url-polyfill/auto';
Expand Down
13 changes: 13 additions & 0 deletions patches/ethjs++ethjs-contract+0.2.2.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
diff --git a/node_modules/ethjs/node_modules/ethjs-contract/package.json b/node_modules/ethjs/node_modules/ethjs-contract/package.json
index 8e58ff5..470d00b 100644
--- a/node_modules/ethjs/node_modules/ethjs-contract/package.json
+++ b/node_modules/ethjs/node_modules/ethjs-contract/package.json
@@ -197,7 +197,7 @@
"lint-staged": {
"lint:eslint": "*.js"
},
- "main": "lib/index.js",
+ "main": "src/index.js",
"name": "ethjs-contract",
"pre-commit": "build",
"repository": {
13 changes: 13 additions & 0 deletions patches/ethjs++ethjs-query+0.3.7.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
diff --git a/node_modules/ethjs/node_modules/ethjs-query/package.json b/node_modules/ethjs/node_modules/ethjs-query/package.json
index fb82d51..503159c 100644
--- a/node_modules/ethjs/node_modules/ethjs-query/package.json
+++ b/node_modules/ethjs/node_modules/ethjs-query/package.json
@@ -2,7 +2,7 @@
"name": "ethjs-query",
"version": "0.3.7",
"description": "A simple query layer for the Ethereum RPC.",
- "main": "lib/index.js",
+ "main": "src/index.js",
"files": [
"dist",
"internals",
58 changes: 58 additions & 0 deletions patches/react-native+0.71.14.patch
Original file line number Diff line number Diff line change
@@ -1,3 +1,61 @@
diff --git a/node_modules/react-native/Libraries/Core/InitializeCore.js b/node_modules/react-native/Libraries/Core/InitializeCore.js
index 1379ffd..340f48d 100644
--- a/node_modules/react-native/Libraries/Core/InitializeCore.js
+++ b/node_modules/react-native/Libraries/Core/InitializeCore.js
@@ -24,26 +24,51 @@

'use strict';

+const Platform = require('../Utilities/Platform');
+
+if (Platform.OS === 'ios' && !global?.HermesInternal) {
+ require('../../../../ses.cjs'); // [email protected]
+ /**
+ * Without consoleTaming: 'unsafe' causes:
+ * - Attempting to define property on object that is not extensible.
+ * Without errorTrapping 'none' causes:
+ * - TypeError: undefined is not a function (near '...globalThis.process.on...')
+ * Without unhandledRejectionTrapping 'none' causes:
+ * - TypeError: globalThis.process.on is not a function. (In 'globalThis.process.on('unhandledRejection', h.unhandledRejectionHandler)', 'globalThis.process.on' is undefined)
+ * overrideTaming 'severe' is ideal (default override?)
+ * Nb: global.process is only partially shimmed, which confuses SES
+ * Nb: All are Unhandled JS Exceptions, since we call lockdown before setUpErrorHandling
+ */
+ repairIntrinsics({ consoleTaming: 'unsafe', errorTrapping: 'none', unhandledRejectionTrapping: 'none', overrideTaming: 'severe' });
+ require('reflect-metadata'); // Vetted shim required to fix: https://github.com/LavaMoat/docs/issues/26
+ hardenIntrinsics();
+}
+
const start = Date.now();

require('./setUpGlobals');
+// require('./setUpDOM'); Introduced in RN v0.72, ensure included when upgrading patch
require('./setUpPerformance');
require('./setUpErrorHandling');
+
require('./polyfillPromise');
+
require('./setUpRegeneratorRuntime');
+
require('./setUpTimers');
require('./setUpXHR');
+
require('./setUpAlert');
require('./setUpNavigator');
require('./setUpBatchedBridge');
require('./setUpSegmentFetcher');
if (__DEV__) {
require('./checkNativeVersion');
- require('./setUpDeveloperTools');
+ require('./setUpDeveloperTools'); // console.log calls visible in Metro from here
require('../LogBox/LogBox').install();
}

-require('../ReactNative/AppRegistry');
+require('../ReactNative/AppRegistry'); // reflect-metadata imported after here causes: https://github.com/LavaMoat/docs/issues/26

const GlobalPerformanceLogger = require('../Utilities/GlobalPerformanceLogger');
// We could just call GlobalPerformanceLogger.markPoint at the top of the file,
diff --git a/node_modules/react-native/ReactAndroid/build.gradle b/node_modules/react-native/ReactAndroid/build.gradle
index 155cb59..053550c 100644
--- a/node_modules/react-native/ReactAndroid/build.gradle
Expand Down
Loading

1 comment on commit 6edfe34

@leotm
Copy link
Member Author

@leotm leotm commented on 6edfe34 Nov 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

android_e2e_test failed again on main exceeding timeout (fixed by a re-run), so could be worth investigating the cause

Please sign in to comment.