TomFrost / Jexl

Javascript Expression Language: Powerful context-based expression parser and evaluator
MIT License
559 stars 90 forks source link

the evaluator allows for arbitrary access to objects' prototypes & invoking of Javascript functions. #92

Open joetIO opened 3 years ago

joetIO commented 3 years ago

Hello Tom. I'm writing an app where untrusted users are able to write their custom Jexl expressions. Expressions get executed on a server. I'm defining few little identifiers & functions mostly primitive types, So i was not worry about security issues. However, While i was developing the software i found that users are able to access any object prototype. e.g I was able to access Function.prototype. Even without any identifier or any function defined on my side:

   Jexl.evalSync('"".toLowerCase["__proto__"]')

And i was able to invoke Javascript functions on asynchronous eval. -was not able to pass any arguments:

   String.prototype.test = () => console.log('called');
   await jexl.eval('{ then: "".test }');

By using Object.prototype.valueOf i should be able to get the output of the functions synchronously. However, It's not working because of a bug in the parser; The parser thinks that valueOf is a Jexl token because it's defined on Object.prototype and the parser uses a Javascript object to store the tokens. however by fixing that bug, valueOf can be used to call any javascript function, like in this case by deleting valueOf from Object.prototype:

    delete Object.prototype.valueOf;
    String.prototype.test = () => 'called';
    jexl.evalSync('"got " + { valueOf: "".test }');

Suggested fixes:

  1. allowing access to properties only if the property is owned by the Object itself. e.g by using Object.hasOwnProperty.
  2. -preventing any binary operation on non-primitive types, So users can't call functions by defining it as a valueOf or toString property-. a better solution would be preventing users from passing functions as Object entries entirely; It will also prevent farther stuff outside jexl like JSON.stringify(jexl.evalSync('{ toJSON: ..... }')).
  3. A fix for the token issue would be by storing tokens, identifiers and functions on Maps instead of Objects or by using Object.hasOwnProperty.

I'm down to make a pull request to fix these issues.

astanciu commented 3 years ago

Any updates on this? Seems like a pretty important security issue..