0

I have an idea for a game where people can type in some simple instructions for their character like player.goLeft() or player.attackInFront() and for that I have people type their code into a text box and then I parse it into eval(). This works well but it also allows people to change their own character object by typing things like player.health = Infinity; or something similar. I have a list of functions I want to allow people to use, but I am unsure how to restrict it to only use them.

I understand that the whole point of not letting people use eval is to avoid accidental cross-site scripting but I am unsure on how else to do this. If you have a suggestion please leave a comment about that.

I asked some people around on what to do and most suggested somehow changing scope(which is something I was not able to figure out) or to add some odd parameter to each function in my code that would be required to be a specific string to execute any function, but that seems hacky and since I am making the game in browser with p5js it would be easy to just inspect element and see what the password is.

basically every character has variable called "instruction" which is just a string of javascript. Then every frame of the game I execute it by doing eval(playerList[i].instruction);

tl;dr, how can I only allow specific function to be executed and not allow any others?

EDIT: I forgot to mention that I also am planning to provide player with information so that people can made code that would adapt to the situation. For example there will be parameter called vision that has vision.front and vision.left etc. These variables would just say if there is an enemy, wall, flower, etc around them in a grid. Some people suggested that I just replace some functions with key words but then it compromises the idea of using if statements and making it act differently.

EDIT 2: Sorry for lack of code in this post, but because of the way I am making it, half of the logic is written on server side and half of it works on client side. It will be a little large and to be completely honest I am not sure how readable my code is, still so far I am getting great help and I am very thankful for it. Thank you to everybody who is answering

Vadim Kim
  • 129
  • 12
  • 2
    You can't really restrict code run via `eval`. You might want to consider a limited scripting language that wraps up the interaction with your underlying code. – Dave Newton Aug 22 '19 at 20:47
  • What most people suggested (changing scope) seems to be reasonable choice. Don't put your functions in the global scope, put them in a module. Run the `eval` code (or even better through `new Function`) in a scope where you can control all variables. (Of course people can still access globals, but that shouldn't do harm). "*which is something I was not able to figure out*" - please show us your code and what you did figure out about it. Only then we can help concretely. – Bergi Aug 22 '19 at 21:17
  • "*I am making the game in browser it would be easy to just inspect element and see what the password is.*" - it always is. You cannot prevent people from cheating on the client side. They can always open the debugger and rewrite your code so that it follows their logic. What makes a difference is only at which length they need to go for that. It definitely shouldn't be possible to cheat from within your code input field, but anything else you can only make harder but never prevent completely. – Bergi Aug 22 '19 at 21:20
  • Hello @Bergi, Thank you for your comments. I get that scope is the way to go but I was unable to figure out how to cut off a function from other things by using scope for example I could make an anonymous function with () =>{} and put eval inside of it. But I am unsure what to do after that? should I make special objects that have very limited functions and only parse them in? – Vadim Kim Aug 22 '19 at 21:30
  • @VadimKim Have a look at [this code](https://stackoverflow.com/a/24032179/1048572) maybe. A function created with `new Function` has the advantage over `eval` that it is created in the global scope, not that of where `eval()` was called. And make sure not to expose your internal stuff (like `playerHealth`) in the global scope, use [an IIFE](https://stackoverflow.com/questions/592396/what-is-the-purpose-of-a-self-executing-function-in-javascript) or module pattern instead. – Bergi Aug 22 '19 at 21:37

3 Answers3

2

Do NOT use eval() to execute arbitrary user input as code! There's no way to allow your code to run a function but prevent eval() from doing the same.

Instead, what you should do is make a map of commands the player can use, mapping them to functions. That way, you run the function based on the map lookup, but if it's not in the map, it can't be run. You can even allow arguments by splitting the string at spaces and spreading the array over the function parameters. Something like this:

const instructions = {
    goLeft: player.goLeft.bind(player),
    goRight: player.goRight.bind(player),
    attackInFront: player.attackInFront.bind(player)
};
function processInstruction(instruction_string) {
    const pieces = instruction_string.split(' ');
    const command = pieces[0];
    const args = pieces.slice(1);
    if (instructions[command]) {
        instructions[command](...args);
    } else {
      // Notify the user their command is not recognized.
    }
};

With that, the player can enter things like goLeft 5 6 and it will call player.goLeft(5,6), but if they try to enter otherFunction 20 40 it will just say it's unrecognized, since otherFunction isn't in the map.

IceMetalPunk
  • 5,476
  • 3
  • 19
  • 26
  • Hello, this looks very much like what I need but I don't really understand example you provided that well. The idea is to analyze their input and break it down into individual commands, and then check if such command is written in my list of instructions? and then bind those instructions to an actually js function? – Vadim Kim Aug 22 '19 at 20:55
  • Basically when a user types `goLeft` in your input the function will do a lookup for the key `goLeft` in the object `instructions`. Never use `eval` in a user-facing system. You're giving the user keys to the mansion – Andrew Aug 22 '19 at 20:58
  • Hello again, I thought about this for a little more and I figured that it will be a problem for me if I do that. I forgot to mention that I also am planning to provide player with information so that people can made code that would adapt to the situation. For example there will be parameter called vision that has vision.front and vision.left etc. These variables would just say if there is an enemy, wall, flower, etc around them in a grid. Some people suggested that I just replace some functions with key words but then it compromises the idea of using if statements and making it act differently – Vadim Kim Aug 22 '19 at 21:00
  • I edited the post, sorry for not mentioning it right away – Vadim Kim Aug 22 '19 at 21:00
  • "*There's no way to allow your code to run a function but prevent eval() from doing the same.*" - sure you can. Just put the function is a scope where your code can access it, but eval cannot. (Of course, the map with instructions to interpret is still a good idea). – Bergi Aug 22 '19 at 21:23
1

This issue sounds similar to the SQL Injection problem. I suggest you use a similar solution. Create an abstraction layer between the users input and your execution, similar to using parameters with stored procedures.

Let the users type keywords such as 'ATTACK FRONT', then pass that input to a function which parses the string, looks for keywords, then passes back 'player.attackInFront()' to be evaluated.

With this approach you simplify the syntax for the users, and limit the possible actions to those you allow.

I hope this isn't too vague. Good luck!

dlbrandt
  • 21
  • 3
0

From your edit, it sounds like you're looking for an object-oriented approach to players. I'm not sure of your existing implementation needs, but it would look like this.

function Player() {
  this.vision = {
    left: '',
    // and so on
  }
}

Player.prototype.updateVisibilities = function() {
  // to modify the values of this.visibility for each player
}

Player.prototype.moveLeft = function() {

}

Don't give the user an arbitrary interface (such as an input textfield that uses eval) to modify their attributes. Make a UI layer to control this logic. Things like buttons, inputs which explicitly run functions/methods that operate on the player. It shouldn't be up to the player as to what attributes they should have.

Andrew
  • 7,201
  • 5
  • 25
  • 34
  • I think the idea behind the game is to allow people write instructions with more or less complex logic, reading state from the environment. A UI layer is not what the OP wants. – Bergi Aug 22 '19 at 21:22
  • @Bergi from his "tl;dr" it sounds like he has a predetermined set of allowed actions and that he's currently giving the user a textfield to manually type anything in. I guess my answer isn't truly an answer, moreso than it is me saying "don't do it that way". – Andrew Aug 22 '19 at 21:31