Implement Local Eval by robik · Pull Request #1515 · facebook/hermes

robik · 2024-09-11T11:58:33Z

Summary

This PR supports local eval in StaticHermes. This is done by persistently filling Frame's Environment value.

I decided to use this approach as it seems to be most stable in the current state. In static hermes environments are serialized into bytecode (it wasn't that easy in previous hermes with ScopeDescs).

I've seen that there is an EvalDataInstruction that serializes VariableScope into bytecode, however, I could not manage to leverage it to build Environment during eval call. I could not access local variables by indices.

This is my first contribution in Hermes ByteCode/interpreter area, so I am very open to be corrected, there might be some side effects I that am not yet aware of.

NOTE: I am yet to ensure it is spec compliant, opening this PR for early verification of solution

Fixes

Feature Request: local eval for SES (Secure EcmaScript) support #957

TODO

Ensure all paths work
- Static Hermes (or at least informs it is not supported)
- Bytecode translation (or at least informs it is not supported)
Check for potential pitfalls
Update docs

Is there anything else to be done?

Test Plan

Before:

> let f = 1; eval('f = 5'); print(f)
1
undefined

After:

> let f = 1; eval('f = 5'); print(f)
5
undefined

tmikov · 2024-09-12T02:13:04Z

@robik thanks for this PR! We discussed it at length in our daily meeting today. Adding such fundamental functionality is very challenging and we love to see in-depth PRs like this.

There are two main problems with this approach.

The first problem, which is shared with the debugger, is that it breaks when block scoping is enabled, because there can be multiple environments in one function. Thus storing the environment in the well known slot is not enough. We are aware of this and we need to solve this in order to enable debugging with block scoping. The solution will apply to local eval() as well. It will be some kind of metadata mapping instruction offsets to active environment.

Anyway, this problem would not block this PR, but I thought you should know the associated complexity.

The second problem is, unfortunately, much more serious. This approach only works when running from source. If you try your example by first compiling to bytecode and then running the bytecode, it will probably throw a JS error (or crash?). In any case, it won't work.

The reason is that all of the metadata mapping variables to environments, etc, is not being serialized to bytecode. It is simply kept in memory by the compiler. When we run directly from bytecode, it is gone.

Again, we are well aware of this limitation. It means, for example, that we can't debug bytecode files. Our debugger only works when running from source. We have long term plans to add serialization.

The question is, are you up to the challenge of implementing this yourself? Serializing all the necessary data structures to bytecode, then de-serializing them for eval() when needed, possibly with some caching? This would be a fundamental improvement of Hermes.

robik · 2024-09-12T08:45:32Z

@robik thanks for this PR! We discussed it at length in our daily meeting today. Adding such fundamental functionality is very challenging and we love to see in-depth PRs like this.

There are two main problems with this approach.

The first problem, which is shared with the debugger, is that it breaks when block scoping is enabled, because there can be multiple environments in one function. Thus storing the environment in the well known slot is not enough. We are aware of this and we need to solve this in order to enable debugging with block scoping. The solution will apply to local eval() as well. It will be some kind of metadata mapping instruction offsets to active environment.

Oh, I missed that. Thanks for the heads up! I think I have few ideas how to tackle this. Are there any unstable areas related to block scoping? Or is it reasonably stable and just hidden behind a flag?

The second problem is, unfortunately, much more serious. This approach only works when running from source. If you try your example by first compiling to bytecode and then running the bytecode, it will probably throw a JS error (or crash?). In any case, it won't work.

Oh right, this is true.

The reason is that all of the metadata mapping variables to environments, etc, is not being serialized to bytecode. It is simply kept in memory by the compiler. When we run directly from bytecode, it is gone.

How would you go about this? Should I base my work on EvalDataInstruction or serialize environments for both eval and debugger? Are there any gotchas with IRGen/BCGen/lowering/optimizations here? If I make it all environment based, should I remove the EvalDataInstruction? Are there any plans for it from your side?

Again, we are well aware of this limitation. It means, for example, that we can't debug bytecode files. Our debugger only works when running from source. We have long term plans to add serialization.

The question is, are you up to the challenge of implementing this yourself? Serializing all the necessary data structures to bytecode, then de-serializing them for eval() when needed, possibly with some caching? This would be a fundamental improvement of Hermes.

I believe I can do this 😁. I will try to work on it next week after solving some ReactNative issues.

What about Static Hermes and Bytecode Translation? Is there anything I should be aware of here?

Should I add the fixes in this PR or as a separate one? Are there any other blockers to get it merged as a partially working solution? (I ask because I'd rather avoid a big PR with all those features)

tmikov · 2024-09-14T16:19:52Z

[...breaks with block scoping...]

Oh, I missed that. Thanks for the heads up! I think I have few ideas how to tackle this. Are there any unstable areas related to block scoping? Or is it reasonably stable and just hidden behind a flag?

Block scoping is an extremely complicated change to the compiler, as it modifies some basic assumptions, like the one-scope per function, and turns scopes into regular values. It is not stable yet and has half-landed. As I mentioned, it doesn't work with the debugger, but we need to tackle that problem, as it will require changing the IR.

The reason is that all of the metadata mapping variables to environments, etc, is not being serialized to bytecode. It is simply kept in memory by the compiler. When we run directly from bytecode, it is gone.

How would you go about this? Should I base my work on EvalDataInstruction or serialize environments for both eval and debugger? Are there any gotchas with IRGen/BCGen/lowering/optimizations here? If I make it all environment based, should I remove the EvalDataInstruction? Are there any plans for it from your side?

Removing EvalDataInstruction is unlikely to be a good approach, since since it is required in order to preserve various data. I don't want to scare you, but I want to emphasize how complicated this undertaking is - it involves understanding all of the compiler data structures, the bytecode file format, how the bytecode file is generated, figuring out how serialize these structures in the file, how to de-serialize them on demand, etc. The reason we haven't done it is that it is that it is hard and likely time consuming.

@avp is working on design documentation for lazy compilation, which is closely related to this. He will probably publish it next week. I recommend waiting for that and getting closely familiar with it before proceeding or making design decisions.

Should I add the fixes in this PR or as a separate one? Are there any other blockers to get it merged as a partially working solution? (I ask because I'd rather avoid a big PR with all those features)

I wouldn't qualify the necessary changes as "fixes" - they will be pretty significant changes to the compiler and runtime, and likely multiple times larger than this PR. I suspect it will be a series of many smaller PRs. So, definitely separate. Please, treat every stacked commit as a separate PR.

We can't merge this one - it will cause people to file bugs that eval() is broken, because it will work in their dev builds, which run from source, but not in production builds.

I want to warn you again, that if you want to do this, it will likely be a lengthy and difficult process. At the end you will emerge as an expert on the Hermes compiler, but it will not be easy. There is no embarrassment and no guilt if you decide to not do this for now. It is also OK if you decide to take it slow.

(Meanwhile, I promise, we haven't forgotten your array diffs!)

tmikov · 2024-09-17T02:56:25Z

Here is the documentation: https://github.com/facebook/hermes/blob/static_h/doc/LazyEvalCompilation.md

- Lock down realm for a Hardened JavaScript (tamper-resistant JS) runtime execution environment (irreversible frozen realm) - Remove ambient authority (unpermitted intrinsics) from global scope (PoLA/PoLP/zero-trust), replace dangerous legacy and non-standard properties (+ known JS engine bugs) - Recursively/transitively (deep) freeze (lock down) ECMA262 (not host) built-in objects (i.e. primordials including hidden intrinsics) and tame (not scuttle) globals - Turn JavaScript system into a Secure ECMAScript system with now enforced OCap (not identity/ACL based) security (an OCap language) to write defensively consistent programs - Protect against prototype poisoning/pollution supply chain attacks from vulnerable/compromised (untrusted/malicious) 3rd party code/dependencies (attempting to access e.g. I/Onetwork,reading/stealing private data) - ECMAScript > ES-strict (static+dynamic i.e. parse+runtime) > Secure ECMAScript (dynamic i.e. runtime) - Use Endo (Agoric) production-grade (prev audited) SES lockdown (Hermes/JSC) shims - SES Proposal partitioned into: Hardened JS (Lockdown) pattern (prerequisite for Compartments), TC39 Compartments (Stage 1), TC39 ShadowRealm (Stage 4 / ES2026) - Supporting Compartments: TC39 Source Phase Imports (Stage 3), TC39 Module Expressions (prev: Blocks) (Stage 2) (e.g. const m = module { export const leet = 1337; }), TC39 Module Declarations (Stage 1) - Noting already: (Moddable) XS (JS) engine - native SES and Compartments; TC53 (IoT) - SES and Compartments; Node.js core --frozen-intrinsics; Salesforce (Lightning Web Security) - SES - github.com/tc39/proposal-compartments, github.com/tc39/proposal-source-phase-imports, github.com/tc39/proposal-module-declarations, github.com/tc39/proposal-module-expressions - github.com/nodejs/node/blob/main/lib/internal/freeze_intrinsics.js, per_context/primordials.js - Prepare realm for code/dependency isolation/sandboxing via new Compartments (stripped non-deterministic Math.random and Date.now) with LavaMoat - Hook into JS bundler (Metro) final stage (resolve -> transform -> serialize) - Polyfills: SES (Hermes) shim, repairIntrinsics (called with RN opts and preserved RN Promise), @react-native/js-polyfills, [no user vetted shims yet] - github.com/LavaMoat/LavaMoat/blob/main/packages/react-native-lockdown/src/index.js - Modules: RN InitializeCore -> (hardenIntrinsics) -> entry file (index.js) - github.com/facebook/react-native/blob/main/packages/community-cli-plugin/src/utils/loadMetroConfig.js serializer, packages/metro-config/src/index.flow.js types - Exclude SES shims from Babel transformation to preserve integrity (endojs/endo#662 Detect if ses is being transformed) - Both Hermes and React Native (for legacy JSC/V8 during InitializeCore) load 'then/promise' polyfill - SES removes the exposed internals `Promise._<onHandle|onReject|noop>` (`Promise._<l|m|n>`) so we restore them after repair as a vetted shim - github.com/LavaMoat/LavaMoat/blob/main/packages/react-native-lockdown/src/repair.js, github.com/endojs/endo/blob/master/docs/lockdown.md - github.com/facebook/hermes/blob/main/utils/promise/index.js, github.com/facebook/react-native/blob/main/packages/react-native/Libraries/Core/polyfillPromise.js - github.com/then/promise/blob/master/src/core.js (`Promise._<onHandle|onReject|noop`), github.com/then/promise/blob/master/src/es6-extensions.js - Evade property override mistake ('Cannot assign to read-only property') via lockdown opt (`overrideTaming: 'severe'`) - i.e. keep prototype immutable, but allow local instances to override methods (e.g. myObj.toString) - endojs/endo#1855 Property override mistake, endojs/endo#2037 Ecosystem Compat (2024-02-05+) - endojs/endo#1511 JSC compat, endojs/endo#1891 Hermes compat - facebook/hermes#957 eval issue, facebook/hermes#1515 eval PR - facebook/hermes#1056 eval issue, facebook/hermes#1571 eval PR - hardenedjs.org, papers.agoric.com/documentation/guides/js-programming/hardened-js.html - github.com/endojs/endo/blob/master/packages/ses/src/lockdown-shim.js, compartment-shim.js, make-evaluate.js (quad backflip), commons.js - github.com/endojs/endo/blob/master/docs/guide.md, github.com/endojs/endo/blob/master/docs/reference.md - github.com/tc39/how-we-work/blob/main/terminology.md, lavamoat.github.io/reference/glossary - github.com/Moddable-OpenSource/moddable/blob/public/xs/sources/xsLockdown.c, xsModule.c, documentation/xs/XS%20Compartment.md Resolve: #1803

- Lock down realm for a Hardened JavaScript (tamper-resistant JS) runtime execution environment (irreversible frozen realm) - Remove ambient authority (unpermitted intrinsics) from global scope (PoLA/PoLP/zero-trust), replace dangerous legacy and non-standard properties (+ known JS engine bugs) - Recursively/transitively (deep) freeze (lock down) ECMA262 (not host) built-in objects (i.e. primordials including hidden intrinsics) and tame (not scuttle) globals - Turn JavaScript system into a Secure ECMAScript system with now enforced OCap (not identity/ACL based) security (an OCap language) to write defensively consistent programs - Protect against prototype poisoning/pollution supply chain attacks from vulnerable/compromised (untrusted/malicious) 3rd party code/dependencies (attempting to access e.g. I/Onetwork,reading/stealing private data) - ECMAScript > ES-strict (static+dynamic i.e. parse+runtime) > Secure ECMAScript (dynamic i.e. runtime) - Use Endo (Agoric) production-grade (prev audited) SES lockdown (Hermes/JSC) shims - SES Proposal partitioned into: Hardened JS (Lockdown) pattern (prerequisite for Compartments), TC39 Compartments (Stage 1), TC39 ShadowRealm (Stage 4 / ES2026) - Supporting Compartments: TC39 Source Phase Imports (Stage 3), TC39 Module Expressions (prev: Blocks) (Stage 2) (e.g. const m = module { export const leet = 1337; }), TC39 Module Declarations (Stage 1) - Noting already: (Moddable) XS (JS) engine - native SES and Compartments; TC53 (IoT) - SES and Compartments; Node.js core --frozen-intrinsics; Salesforce (Lightning Web Security) - SES - github.com/tc39/proposal-compartments, github.com/tc39/proposal-source-phase-imports, github.com/tc39/proposal-module-declarations, github.com/tc39/proposal-module-expressions - github.com/nodejs/node/blob/main/lib/internal/freeze_intrinsics.js, per_context/primordials.js - Prepare realm for code/dependency isolation/sandboxing via new Compartments (stripped non-deterministic Math.random and Date.now) with LavaMoat - Hook into JS bundler (Metro) final stage (resolve -> transform -> serialize) - Polyfills: SES (Hermes) shim, repairIntrinsics (called with RN opts and preserved RN Promise), @react-native/js-polyfills, [no user vetted shims yet] - github.com/LavaMoat/LavaMoat/blob/main/packages/react-native-lockdown/src/index.js - Modules: RN InitializeCore -> (hardenIntrinsics) -> entry file (index.js) - github.com/facebook/react-native/blob/main/packages/community-cli-plugin/src/utils/loadMetroConfig.js serializer, packages/metro-config/src/index.flow.js types - Exclude SES shims from Babel transformation to preserve integrity (endojs/endo#662 Detect if ses is being transformed) - Both Hermes and React Native (for legacy JSC/V8 during InitializeCore) load 'then/promise' polyfill - SES removes the exposed internals `Promise._<onHandle|onReject|noop>` (`Promise._<l|m|n>`) so we restore them after repair as a vetted shim - github.com/LavaMoat/LavaMoat/blob/main/packages/react-native-lockdown/src/repair.js, github.com/endojs/endo/blob/master/docs/lockdown.md - github.com/facebook/hermes/blob/main/utils/promise/index.js, github.com/facebook/react-native/blob/main/packages/react-native/Libraries/Core/polyfillPromise.js - github.com/then/promise/blob/master/src/core.js (`Promise._<onHandle|onReject|noop`), github.com/then/promise/blob/master/src/es6-extensions.js - Evade property override mistake ('Cannot assign to read-only property') via lockdown opt (`overrideTaming: 'severe'`) - i.e. keep prototype immutable, but allow local instances to override methods (e.g. myObj.toString) - endojs/endo#1855 Property override mistake, endojs/endo#2037 Ecosystem Compat (2024-02-05+) - endojs/endo#1511 JSC compat, endojs/endo#1891 Hermes compat - facebook/hermes#957 eval fn issue, facebook/hermes#1515 PR - facebook/hermes#1056 with statement issue, facebook/hermes#1571 eval PR - hardenedjs.org, papers.agoric.com/documentation/guides/js-programming/hardened-js.html - github.com/endojs/endo/blob/master/packages/ses/src/lockdown-shim.js, compartment-shim.js, make-evaluate.js (quad backflip), commons.js - github.com/endojs/endo/blob/master/docs/guide.md, github.com/endojs/endo/blob/master/docs/reference.md - github.com/tc39/how-we-work/blob/main/terminology.md, lavamoat.github.io/reference/glossary - github.com/Moddable-OpenSource/moddable/blob/public/xs/sources/xsLockdown.c, xsModule.c, documentation/xs/XS%20Compartment.md Resolve: #1803

- Lock down realm for a Hardened JavaScript (tamper-resistant JS) runtime execution environment (irreversible frozen realm) - Remove ambient authority (unpermitted intrinsics) from global scope (PoLA/PoLP/zero-trust), replace dangerous legacy and non-standard properties (+ known JS engine bugs) - Recursively/transitively (deep) freeze (lock down) ECMA262 (not host) built-in objects (i.e. primordials including hidden intrinsics) and tame (not scuttle) globals - Turn JavaScript system into a Secure ECMAScript system with now enforced OCap (not identity/ACL based) security (an OCap language) to write defensively consistent programs - Protect against prototype poisoning/pollution supply chain attacks from vulnerable/compromised (untrusted/malicious) 3rd party code/dependencies (attempting to access e.g. I/Onetwork,reading/stealing private data) - ECMAScript > ES-strict (static+dynamic i.e. parse+runtime) > Secure ECMAScript (dynamic i.e. runtime) - Use Endo (Agoric) production-grade (prev audited) SES lockdown (Hermes/JSC) shims - SES Proposal partitioned into: Hardened JS (Lockdown) pattern (prerequisite for Compartments), TC39 Compartments (Stage 1), TC39 ShadowRealm (Stage 4 / ES2026) - Supporting Compartments: TC39 Source Phase Imports (Stage 3), TC39 Module Expressions (prev: Blocks) (Stage 2) (e.g. const m = module { export const leet = 1337; }), TC39 Module Declarations (Stage 1) - Noting already: (Moddable) XS (JS) engine - native SES and Compartments; TC53 (IoT) - SES and Compartments; Node.js core --frozen-intrinsics; Salesforce (Lightning Web Security) - SES - github.com/tc39/proposal-compartments, github.com/tc39/proposal-source-phase-imports, github.com/tc39/proposal-module-declarations, github.com/tc39/proposal-module-expressions - github.com/nodejs/node/blob/main/lib/internal/freeze_intrinsics.js, per_context/primordials.js - Prepare realm for code/dependency isolation/sandboxing via new Compartments (stripped non-deterministic Math.random and Date.now) with LavaMoat - Hook into JS bundler (Metro) final stage (resolve -> transform -> serialize) - Polyfills: SES (Hermes) shim, repairIntrinsics (called with RN opts and preserved RN Promise), @react-native/js-polyfills, [no user vetted shims yet] - github.com/LavaMoat/LavaMoat/blob/main/packages/react-native-lockdown/src/index.js - Modules: RN InitializeCore -> (hardenIntrinsics) -> entry file (index.js) - github.com/facebook/react-native/blob/main/packages/community-cli-plugin/src/utils/loadMetroConfig.js serializer, packages/metro-config/src/index.flow.js types - Exclude SES shims from Babel transformation to preserve integrity (endojs/endo#662 Detect if ses is being transformed) - Both Hermes and React Native (for legacy JSC/V8 during InitializeCore) load 'then/promise' polyfill - SES removes the exposed internals `Promise._<onHandle|onReject|noop>` (`Promise._<l|m|n>`) so we restore them after repair as a vetted shim - github.com/LavaMoat/LavaMoat/blob/main/packages/react-native-lockdown/src/repair.js, github.com/endojs/endo/blob/master/docs/lockdown.md - github.com/facebook/hermes/blob/main/utils/promise/index.js, github.com/facebook/react-native/blob/main/packages/react-native/Libraries/Core/polyfillPromise.js - github.com/then/promise/blob/master/src/core.js (`Promise._<onHandle|onReject|noop`), github.com/then/promise/blob/master/src/es6-extensions.js - Evade property override mistake ('Cannot assign to read-only property') via lockdown opt (`overrideTaming: 'severe'`) - i.e. keep prototype immutable, but allow local instances to override methods (e.g. myObj.toString) - endojs/endo#1855 Property override mistake, endojs/endo#2037 Ecosystem Compat (2024-02-05+) - endojs/endo#1511 JSC compat, endojs/endo#1891 Hermes compat - facebook/hermes#957 eval fn issue, facebook/hermes#1515 PR - facebook/hermes#1056 with statement issue, facebook/hermes#1571 PR - hardenedjs.org, papers.agoric.com/documentation/guides/js-programming/hardened-js.html - github.com/endojs/endo/blob/master/packages/ses/src/lockdown-shim.js, compartment-shim.js, make-evaluate.js (quad backflip), commons.js - github.com/endojs/endo/blob/master/docs/guide.md, github.com/endojs/endo/blob/master/docs/reference.md - github.com/tc39/how-we-work/blob/main/terminology.md, lavamoat.github.io/reference/glossary - github.com/Moddable-OpenSource/moddable/blob/public/xs/sources/xsLockdown.c, xsModule.c, documentation/xs/XS%20Compartment.md Resolve: #1803

feat: implement local eval

b58a334

facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Sep 11, 2024

leotm mentioned this pull request Dec 17, 2024

SES lockdown Hermes compat tracking issue endojs/endo#1891

Closed

27 tasks

leotm mentioned this pull request Mar 7, 2025

feat(ses): update lockdown init and enable Hermes VM endojs/endo#2723

Merged

23 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Local Eval#1515

Implement Local Eval#1515
robik wants to merge 1 commit into
facebook:static_hfrom
robik:robik/local-eval-refs

robik commented Sep 11, 2024 •

edited

Loading

Uh oh!

tmikov commented Sep 12, 2024

Uh oh!

robik commented Sep 12, 2024 •

edited

Loading

Uh oh!

tmikov commented Sep 14, 2024

Uh oh!

tmikov commented Sep 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

robik commented Sep 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Fixes

TODO

Test Plan

Uh oh!

tmikov commented Sep 12, 2024

Uh oh!

robik commented Sep 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tmikov commented Sep 14, 2024

Uh oh!

tmikov commented Sep 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

robik commented Sep 11, 2024 •

edited

Loading

robik commented Sep 12, 2024 •

edited

Loading