NeilFraser / JS-Interpreter

A sandboxed JavaScript interpreter in JavaScript.
Apache License 2.0
1.98k stars 353 forks source link

Custom toString() on objects not supported #194

Closed atifsyedali closed 4 years ago

atifsyedali commented 4 years ago

Steps to reproduce:

  1. Enter the following in index.html:
var a = {
 x: 42,
 toString: function() {
   return "world";
 }
};

alert("Hello " + a + " " + a.x);

Expected (test with Node 6.x): Hello world 42

Actual: Hello [object Object] 42

NeilFraser commented 4 years ago

Yes, implicit calls to toString and valueOf are explicitly listed as one of the limitations of the JS-Interpreter: https://neil.fraser.name/software/JS-Interpreter/docs.html

Architecturally, it would be extremely difficult to implement this. Sorry.

atifsyedali commented 4 years ago

Ok after diving into the code I think I see why. The interpreter overrides the toString method to provide a pseudo implementation, and lets the host VM call into that toString method (and like other binary operations too). It's clever as it avoids you having to implement the actual low-level execution machinery yourself. But it means toString, binary operations and low-level exec stuff are all synchronous and you can't connect the async nature of stepper execution to the low-level host execution.

I still desperately need the toString though, so I am going to fork it and add in this line to the toString() which allows me to use a toString supplied by a nativeFunction at the very least.

  } else if (this.class === "Object" && this.properties.toString && this.properties.toString.nativeFunc) {
    // at the very least support a synchronous native function call.
    return this.properties.toString.nativeFunc();
  }

Up to you if you want to add this in to this repo.

NeilFraser commented 4 years ago

The reason this will remain unimplemented in this repo is that the step-by-step nature of the interpreter fails for implicitly called toString methods. So if a user-defined toString contains an infinite loop, the moment one steps into a call of this function, the browser's JS runtime will crash. As a sandbox, I'd rather have the feature missing, than have a violation of the sandbox.

Of course your priorities and use-cases may be completely different, so for you this might make sense. But just be aware that it violates the main goal of the JS-Interpreter: safe execution of potentially malicious JavaScript.