Skip to content
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

Allow correlation of scopeProxy to compartment, as workaround for scopeProxy leakage #622

Closed
kumavis opened this issue Mar 17, 2021 · 4 comments · Fixed by #623
Closed

Comments

@kumavis
Copy link
Member

kumavis commented Mar 17, 2021

update

i realized this proposal is unnecessary, but thread may be informational

LavaMoat issue summary

LavaMoat needs to be able to support arbitrary platform apis that are this-value specific. eg:

const trueGlobalThis = globalThis
// this, defined in sloppy mode, approximates the behavior of `setTimeout` in chrome
trueGlobalThis.abc = function(){ if (this !== trueGlobalThis) throw new Error("boom")

currently it looks like this.
it explodes because abc's this-value is scopeProxy

require('ses/lockdown')
lockdown()
const compartment = new Compartment()
// abc is copied onto compartment.globalThis here
copyEndowmentsToCompartment(policy, trueGlobalThis, compartment.globalThis)
compartment.evaluate('abc()') // < ----- explodes here

I'm not sure what non-shim SES's behavior is, but i would expect the this-value to be the compartment.globalThis

copyEndowmentsToCompartment already wraps functions and getters in order to "unwrap" (membrane-like) the this-value from compartment.globalThis to trueGlobalThis in order to satisfy the trueGlobalThis.abc implementation requirements above and similar requirements found in the wild, especially in chrome (window.document, window.setTimeout)

we could similarly unwrap the this-value from scopeProxy to compartment.globalThis if we knew how to verify the scopeProxy came from the compartment.

const newThis = compartment.isScopeProxy(thisValue) ? compartment.globalThis : thisValue

proposal

as a workaround for scopeProxy leakage #31,
something like this

Compartment.prototype.isScopeProxy = function (value) { return value === scopeProxy }
@kumavis
Copy link
Member Author

kumavis commented Mar 17, 2021

‼️ actually we can just use the scopeProxy leak to get a hold of it and do the check ourselves 🤪

@kumavis
Copy link
Member Author

kumavis commented Mar 17, 2021

mmmm, scopeProxy is unique per-evaluation 😿

@erights
Copy link
Contributor

erights commented Mar 23, 2021

I'm not sure what non-shim SES's behavior is, but i would expect the this-value to be the compartment.globalThis

The proposed behavior would parallel JS's actual behavior for the real first global, the global of the start compartment. The with trick is, after all, only trying to emulate the JS semantics of global access, but for a different object as the global. Illustrating JS's global semantics on this issue:

function foo() { "use strict"; return this; }

// This is the global behavior we wish to emulate.
foo(); // undefined

const x = {};

// Curiously, this emulates it, which does suggest a technique which would work in theory
// but is too dangerous. Hmm I might try it on a branch just to see what it looks like.
with(x) { foo(); } // undefined

x.foo = foo;

// Given our current technique, whose hazards we have been living with and presumably
// understand and have made safe enough, this is the outcome we seem to be stuck with
// according to #31 
with(x) { foo() } // x

valueOf() // throws TypeError "Cannot convert undefined or null to object"

So, using your __isKnownScopeProxy__ trick to emulate that behavior would be nice, especially if it is transparent. Doubly good if it is so transparent it cannot be worked around and can enforce the denial of access to the scopeProxy. From your explanation above, it seems the next thing for me to understand is copyEndowmentsToCompartment. What does it do? What should it do?

@erights
Copy link
Contributor

erights commented Mar 23, 2021

I updated the code above to insert the relevant comments. The key is the first one. The behavior we will spec, and which therefore we'd ideally like to emulate, is that this of a function looked up on the global is undefined.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants