-
Notifications
You must be signed in to change notification settings - Fork 72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SES lockdown Hermes compat tracking issue #1891
Comments
My understanding of this issue is that we’re tracking integration problems with Hermes and some of these may motivate recommendations to change the SES shim to broaden its portability. (I’m driving similar work with XS.) You also are getting by with some transformations and manual edits of the SES shim that might upstream. @leotm Do you plan to propose changes? One of the things I am thinking of doing for XS is elaborate the {
"exports": {
"hermes": {},
"xsnap": {},
"default": {},
}
} |
Exactly ^ renamed to tracking issue, was initially only planning on running a local codemod once functional, but a new separate export (non-async SES shim) for Hermes makes sense 👍 like you're thinking for XS |
Updated description with deeper dive on |
Briefly demo'd MetaMask/metamask-mobile#8009, a minimally transformed [email protected] wip that runs on Hermes (latest React Native default JS engine)
here's the omnibus of issues/mods below, while figuring which parts we definitely need (vs can skip/remove in a codemod) cc @erights would appreciate any thoughts
Bundling (Metro, release)
error: async functions are unsupported
loadWithoutErrorAnnotation
memoizedLoadWithErrorAnnotation
load
drainQueue
([email protected])asyncTrampoline
(ses@master)error: async generators are unsupported
async function* AsyncGeneratorFunctionInstance() {}
Runtime
// assertDirectEvalAvailable(); // SES TypeError
eval
for SES (Secure EcmaScript) support facebook/hermes#957with
statement for SES (Secure EcmaScript) support facebook/hermes#1056// addIntrinsics(tameRegExpConstructor(regExpTaming)); // SES TypeError
// addIntrinsics(tameSymbolConstructor()); // Metro exception, RN polyfill error guard Fatal Error
// addIntrinsics(getAnonymousIntrinsics()); // SES TypeError
// completePrototypes(); // SES TypeError
// whitelistIntrinsics(intrinsics, markVirtualizedNativeFunction); // SES TypeError
// hardenIntrinsics(); // no RN app hangs loading, SES TypeError in Hermes VM
__hardenTaming__: 'unsafe'
isn't sufficient for the protection we requireTested across bundled Hermes for RN
Nb: latest minor version tags above, side-by-side with Hermes tarballs downloaded during build
async functionsNb:
async function* a() {};
alone won't emit an errorbut using/referencing it and beyond
const b = a;
willencountered bundling the app with Metro (considering Re.Pack)
Since Hermes
async
andawait
support is In Progress (let alone async generators)so Hermes doesn't support async arrow functions directly
but we're getting indirect support later via @babel/preset-env
which lowers them to generator fns or plain fns using regenerator (relies on regenerator-runtime)
nb: [email protected] includes these 37 babel plugins
If so, re-write to Promises, otherwise skip parts or remove and refactor, or is there a better solution?
TODO: check refactor from async arrow fns to async fns; check latest bundled Hermes versions of RNNo longer an issue on bundled Hermes for RN 0.71.17+ (i.e. not a custom build from source)
assertDirectEvalAvailable
Since
eval
is Excluded From Supporteval
for SES (Secure EcmaScript) support facebook/hermes#957tameRegExpConstructorno RegExp[Symbol.species] descriptor
,no stack
ses.cjs#4472:
TypeError('no RegExp[Symbol.species] descriptor');
This is because it is Excluded From Support on Hermes
For a good reason it seems, noting both warnings
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol/species
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/@@species
So we likely want this part skipped or tameRegExpConstructor option to skip (plus remaining changes, then upstream)
fixed in
tameSymbolConstructor
property is not configurable
,no stack
/at [object CallSite]
ses.cjs#9481:
defineProperties(SharedSymbol, descs);
We can fix this simply by omitting
configurable: true
on Hermesin order to tame the Symbol constructor (used on constructed compartments),
we attempt to temporarily make all props
configurable: true
so that we can whitelist (remove non-standard) props, then harden them (back to non-configurable)
Hermes has 15
Symbol
props on all versions (contrasted to e.g. Node with 18)on metamask-mobile (Android)
when we observe
Object.getOwnPropertyDescriptor(originalDescsEntries, name)?.configurable
we get 15 non-true props: 1
false
prop (length
), 14undefined
nb: on Hermes v0.12.0 built from scratch
(i built/ran Hermes from scratch to observe above, since Hermes via eshost-cli on m2 is unsupported via jsvu(mac64arm) and doesn't fetch via esvu(darwin-arm64) and built Hermes via eshost complains
hermes: Unknown command line argument '-fenable-tdz'
and Hermes playground only prints[object Object]
- but thx for demo'ing @gibson042, jealous how it worked so eloquently on your machine :p)nb: some Hermes Language Features
Symbol.species
and its interactions with JS library functionsSymbol.unscopables
(Hermes does not supportwith
)Symbol.prototype.description
(it's not fully spec-conformant yet.Symbol().description
should beundefined
but it's currently''
)[Symbol.iterator]
)nb: our
tameSymbolConstructor
fnnb: a
SymbolTaming
lockdown option doesn't exist yet (same with FunctionTaming ontameFunctionConstructors
)solutions
packages/ses/src/tame-symbol-constructor.js
)packages/ses/src/lockdown.js#L276
)safe
(current implementation)unsafe
(skip taming, none at all)extra
The ses shim is constructed to _eventually_ enable vetted shims...
10mo ago now we have vetted shimssecurity considerations pending further thought
getAnonymousIntrinsics
Conflicting definitions of %InertAsyncFunction%
,no stack
ses.cjs#3688:
TypeError( `Conflicting definitions of ${name}`);
Not present in
debugger
statements working in Flipper > Hermes Debugger (RN)Present in
debugger
statements broken in Flipper > Hermes Debugger (RN)commenting
results in SES continuing to lockdown
resulting in a fully functional React Native app
but at what security cost
completePrototypes
lockdown.prototype property not whitelisted
,no stack
ses.cjs#3730:
TypeError( `${name}.prototype property not whitelisted`)
to repro, uncomment
endo/packages/ses/scripts/hermes.sh
Line 38 in cdd2dad
whitelistIntrinsics
Unexpected intrinsic intrinsics.isFinite.__proto__ at %FunctionPrototype%
,no stack
ses.cjs#3953:
TypeError( `Unexpected intrinsic ${path}.__proto__ at ${protoName}`)
isFinite
appears to be the first of many intrinsicsNo more SES TypeErrors thrown after whitelisting intrinsics
hardenIntrinsics
No errors visible, but the React Native app hangs
__hardenTaming__: 'unsafe'
(default safe) fixes the issue, but is not a viable solutionhttps://github.com/endojs/endo/blob/master/packages/ses/docs/lockdown.md#__hardentaming__-options
The Hermes VM throws
The text was updated successfully, but these errors were encountered: