JavaScript Interpreter Safety?

Discussion in 'Plugin Development' started by cakenggt, Jan 28, 2015.

Thread Status:
Not open for further replies.
  1. Offline

    cakenggt

    I am planning on including a JavaScript interpreter in my plugin which will allow users to write limited JavaScript. I want to do this as safely as possible, which means preventing them from doing things like calling System.exit() and whatnot. I want to use javax.script to do this.

    My original plan is to prevent users from importing classes or packages by setting those methods to null. Then, the only object I give them access to is an object which will never return objects which could be used to reach outside and do anything dangerous to the computer or to the server.

    An example of my code is as follows, obviously not a direct paste from the plugin but just to show the idea:
    Code:
    public class Node{
    
       private int x =5;
    
       public int y =8;
    
       private int position =0;
    
       public int getPosition(){return position;}
    
       private String getString(){return"hello";}
    
       public String getBing(){return"bing";}
    
    }
    
    public void executeCommand(String command){
       ScriptEngineManager factory =new ScriptEngineManager();
       ScriptEngine engine = factory.getEngineByName("JavaScript");
       Node node =newNode();
       engine.put("node", node);
       try {
           engine.eval("importPackage = null; importClass = null");
           engine.eval(command);
       } catch (ScriptException e1){
           e1.printStackTrace();
       }
    }
    With the code above, assuming I only put methods in my Node class which return types that contain no references to unsafe outside things (only returning things such as String, Bukkit Vector, etc.), are there still dangers to using this approach?
     
  2. Offline

    _Filip

    1. You still have to declare the variables in the script engine, no? It won't recognize "System" and will throw an exception.
    2. Please don't initialize the script engine on every single invocation of the execute method...
    3. You should execute the JS on a different thread.
     
  3. Offline

    cakenggt

    1. Yes, you are right. That's why I'm only giving them the one variable and preventing them from importing classes or packages.
    2. If I don't initialize the script engine anew every time, won't different users have access to the same variables? What if a second user runs a command while the first's command is still running. Wouldn't that mean that the first user's node variable would be overwritten by the second's?
    3. I agree, I am creating a new thread each time the user executes a command.

    The command string is meant to encompass the entire program run, not just one line of it.
     
    Last edited: Jan 28, 2015
  4. Offline

    _Filip

    @cakenggt Instead of making it a command have you thought of making it a clickable sign?
    I think that it would be much more interesting and cleaner that writing the multi-statement code on a single line in a chat prompt.
     
  5. Offline

    cakenggt

    I'm actually having it take text from a book, add all the pages together, and then put it in the lore of an item, which can be activated later on.
     
Thread Status:
Not open for further replies.

Share This Page