Securing user defined javascript function to protect node.js server

Question

I'm making a math-teaching webpage (NodeJS backend and Angular frontend). I want a special kind of users (creator) to create mathematical exercises. One of these exercises can look like this:

Marie has ${nums[0]} oranges and ${nums[1]} apples. How many fruits does she have?

Now I want the creator to write a number generating function like this:

const generate = () => {
 const nums = new Array(2).fill(0).map(e => Math.floor(Math.random() * 10)
 return { nums: nums, answer: nums.reduce((p, c) => p + c, 0) }
}

This function should be send to the server and stored. When the user want to try the test, the question should be executed on the server. What should I do to protect the server from malicious code like:

const generate = () => {
 process.exit()
}

• Mark: There are some Sandbox modules that are made for this. For example: vm2
• Konrad: @MarkMeyer Thank you for tips. vm2 doesn't support memory limits. I'm testing pitboss-ng and it works quite fine.

Answer 1

Really short answer, this is never really safe for the server. It is impossible to prove that a program is safe. There are mitigations such as sandboxing that help, but it is ultimately always a risk. For this application, possibly an unnecessary one.

Consider some way of communicating the formula that does not require exec. One way might be to send an abstract syntax tree of some sort, or to parse the mathematical expression.

This npm package seems promising. Fill a math expression string template the same way you fill the written question template. It might be necessary to provide another object to define what random numbers are needed and map them to names for use in the templates. math-expression-evaluator

• Konrad: The problem with the mathematical expression is that I need variables relativity. Let's say that the question is: "Carla, Alex and Camille have x candies. They want to divide candies evenly, so each of them will have the same number of candies. How many candies should take each of them?" The problem with that kind of exercises is that number of people should be dividable without reminder by x. it is hard to calculate that using math formula (I don't even say about random names generator). Maybe I just don't know how to use it right.
• le3th4x0rbot: @Konowy Maybe where you define random values allow an expression to be run on them at generation time. In this example it might be like {name:'apples' 'type:'int', min:2, max:20, exp:'2*rand'}.
• le3th4x0rbot: Then the problem template might be "Carla, Alex and Camille have ${apples} apples.[...]". The answer expression would be ${apples} / 2.
• le3th4x0rbot: Another useful feature might be to allow named random values to be created from previously generated ones like: {name:'sticks' type:'calc', exp:'5*apples'}. This way it would be easy to define values that have mathematical expression relationships to each other.
• le3th4x0rbot: Maybe still another answer might be to implement some kind of code running service in something like AWS Lambda with sufficiently restricted IAM permissions. This would offload the main isolation concerns to a product that is not your problem. Make sure to set good timeouts and reasonable per user usage limits in this case. Use a separate AWS account if other services are hosted on AWS, they guarantee at least VM level separation between accounts.

Answer 2

This will not be safe for the server/client.

Not sure this is the correct way or not. but you can restrict global variables access

const generate1 = () => {
 process.exit();
}
const generate2 = () => {
 const nums = new Array(2).fill(0).map(e => Math.floor(Math.random() * 10));
 return {
 nums: nums,
 answer: nums.reduce((p, c) => p + c, 0)
 }
}
const funcStr1 = generate1.toString();
const funcStr2 = generate2.toString();
//get all global variables key from 'global' and check it exist or not
// const globals = Object.keys(global); // will return same result as below remove keys which you want to allow
const globals = ['DTRACE_NET_SERVER_CONNECTION',
 'DTRACE_NET_STREAM_END',
 'DTRACE_HTTP_SERVER_REQUEST',
 'DTRACE_HTTP_SERVER_RESPONSE',
 'DTRACE_HTTP_CLIENT_REQUEST',
 'DTRACE_HTTP_CLIENT_RESPONSE',
 'COUNTER_NET_SERVER_CONNECTION',
 'COUNTER_NET_SERVER_CONNECTION_CLOSE',
 'COUNTER_HTTP_SERVER_REQUEST',
 'COUNTER_HTTP_SERVER_RESPONSE',
 'COUNTER_HTTP_CLIENT_REQUEST',
 'COUNTER_HTTP_CLIENT_RESPONSE',
 'global',
 'process',
 'Buffer',
 'clearImmediate',
 'setImmediate',
 // 'clearInterval',
 // 'clearTimeout',
 // 'setInterval',
 // 'setTimeout'
];
const hasGlobal1 = globals.some((g) => funcStr1.includes(`${g}.`));
const hasGlobal2 = globals.some((g) => funcStr2.includes(`${g}.`));
console.log('generate1 has global', hasGlobal1);
console.log('generate2 has global', hasGlobal2);

• Yair Nevet: Globals abusement usage might just be one of the cases, what about infinite loops or excessive memory consumption?
• Rahul Sharma: @YairNevet that's true, I mentioned This will not be safe for the server. the only global variable you can restrict.
• Yair Nevet: what I mentioned is neither safe for the server or the client side
• Konrad: Your answer is what I considered before, but it's not working as expected because Array.prototype.includes can also catch strings and user defined variables. What I was thinking was rather to overwrite all globals before execution and reassign them again after that,. As @YairNevet said, infinite loops is a big problem here.