nx-js/compiler-util

this === window ?

Closed this issue · 9 comments

Not sure if there is a problem or not, but this seems to resolve to window, so i guess i can access global stuff through that. Not sure if this is something that is ok or not or whether a .call(null or .apply(null would help...

the blog article https://blog.risingstack.com/writing-a-javascript-framework-sandboxed-code-evaluation/ has some typos too, so copy/pasting the code to the devtools doesnt always work.

Another thing i was thinking about is, that maybe someone could "pass in" an object that contains a has or get method which is "destructured" by the with (...) { ... } statement and maybe can cause problems too?

Hi!

Thanks for all these observations, gonna check them all tomorrow.

Hi!

  1. This really resolved to the global object (window), because the functions were in global space. Fixed it with your suggestion: code.call(sandboxProxy, sandboxProxy). Now this inside sandboxed code points to the sandbox. Do you think it would make more sense to set it to null?
  2. I am checking the typos.
  3. I am not sure I understand this one. There are two parts here: the sandbox object (secure, completely controlled by the owner) and the code string (can be injected). If a developer does crazy stuff in the sandbox (like exposing global inside getters) it's his/her own fault. If the sandbox itself doesn't expose globals but the code string can somehow get access to them, then it's the fault of nx-compile. Or did you mean creating objects with getters inside the code string?

Thx for all the feedback!

i dont know, it probably doesnt matter, as long as its sandboxed.

Regarding 3. - i'm also not so sure and just guessing from a gut feeling, but:

  • What would happen if somebody would define a has and a get function inside the source code that will be put into the sandbox, so that they maybe affect your designed has and get functions by somehow shadowing the - so that when they are supposed to be called and prevent access to the global object, the ones defined in the source allow the access to the global object instead.
    ...maybe this wouldnt work, but maybe it would?

Oh, I got it.

It won't cause issues. has and get are both part of the Proxy. Firstly I don't think they are actually added as properties to the Proxy object. Secondly it is not possible to figure this out, since even if they are added as properties Proxies are transparent. Meaning that if you have const proxy = new Proxy(obj, {get, has}) and try to do proxy.get, it will always look for get inside obj instead of proxy.

This is the theory, I also tested it quickly to make sure. I think I will include a test for this case to make it clear.

(Side note: if you are interested, someone else found another issue that I am currently struggling with: https://www.reddit.com/r/javascript/comments/4xz2n8/writing_a_javascript_framework_sandboxed_code/)

This fix is on master with documentation and test coverage. I will close this issue now.

I wasn't thinking about proxy.get or obj.get (or .has) ...
What if the source code defines a function has () { return false} ... will they maybe be called when proxy.has is used to look up stuff instead of

function has (target, key) {
  if (isAllowedGlobal(key)) {
    return Reflect.has(target, key)
  }
  return true
}

Nope, there is no way to access or overwrite the Proxy traps (has and get) from inside the code.

ok perfect :-) thats awesome. I also read the reddit thing. Will follow this repo.
It's nice to have a vm in ~20lines of code
👍

Great (:
I am glad you like it. Thx for the comments.