Math and Number functions

[ramirobg94]

Im trying to execute the following expressions: evalExpr("Number('1') >= 101") // ERROR evalExpr("Math.max(1,10)") // ERROR

In both cases the system triggers an error because Math and Number are not found so I need to pass

evalExpr("Number('1') >= 101", {Number: (x) => Number(x)}) // false evalExpr("Number('1') >= 101", {Math}) // 1

but If I execute evalExpr("'A'.toLowerCase()") // 'a'

What is the best way to solve this? Do a PR to the library (no idea about where) adding Math and Number or pass as context all the time?

Thanks,

[6utt3rfly]

@ramirobg94 I was originally thinking that it's better to keep the evaluation context empty by default to limit code execution. It's easy enough for the user to set the allowable methods, like:

{
  Date,
  Array,
  Object,
  encodeURI,
  decodeURI,
  isFinite,
  isNaN,
  JSON,
  Math,
  parseFloat,
  parseInt,
  RegExp,
  ...customFunctionsAndProperties,
}

(and I'm probably missing some in that list)... I'm not sure it's necessary to be included by default in this module? But I guess if they were included by default, they could also be overridden to remove? 🤷🏻‍♀️

[ramirobg94]

In my opinion, I think that is better than the function includes this because at the end we are evaluating JS so imagine that you are opening and script and you are running JS and you have to import Date, Array, Object ... prove that is your message: "and I'm probably missing some in that list" this makes me feel "unsafe" and when I use a library is because the library gives me all of the stuff ready.

At least one intermediate solution, with some clarification note in the readme, should expose a "basicJsFunctions" context and allow the user to add this

[6utt3rfly]

@ramirobg94 - I've updated the readme ... I checked a few other similar projects and none that I found support JS functions by default. Some projects disable function execution by default too. Also, using the spread operator could make the use of the assignment operator break for some use cases and I wouldn't want to mutate the context argument otherwise. Maybe it's best to wait to see if this is a commonly requested update?

[github-actions[bot]]

:tada: This issue has been resolved in version 1.5.1 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket:

[ianroberts]

Adding Object to the context makes it trivially easy to bypass the jse-eval checks that block access to protoype and __proto__, letting the expression pollute the global prototypes of Object, Array, etc., so I would leave it opt-in if you want to be (less-in)secure-by-default:

Object.assign(Object.getPrototypeOf([]), { toString: () => "Replaced Array.prototype.toString" })

Not as dangerous as it could be given we only have arrow function expressions (so my replacement toString "method" can't see the array it was called on as this) but still potential to cause mischief.