Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
joetIO opened this issue Oct 1, 2020 · 1 comment

Comments

@joetIO
Copy link

joetIO commented Oct 1, 2020

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.

@joetIO joetIO changed the title the evaluator allows for arbitrary access to objects' prototypes & invoking of Javascript's functions. bug with tokens having the same name as object.prototype members the evaluator allows for arbitrary access to objects' prototypes & invoking of Javascript functions. bug with tokens having the same name as object.prototype members Oct 1, 2020
@joetIO joetIO changed the title the evaluator allows for arbitrary access to objects' prototypes & invoking of Javascript functions. bug with tokens having the same name as object.prototype members the evaluator allows for arbitrary access to objects' prototypes & invoking of Javascript functions. bug with tokens having the same names as object.prototype members Oct 1, 2020
@joetIO joetIO changed the title the evaluator allows for arbitrary access to objects' prototypes & invoking of Javascript functions. bug with tokens having the same names as object.prototype members the evaluator allows for arbitrary access to objects' prototypes & invoking of Javascript functions. and a bug with tokens having the same names as object.prototype members Oct 1, 2020
@joetIO joetIO changed the title the evaluator allows for arbitrary access to objects' prototypes & invoking of Javascript functions. and a bug with tokens having the same names as object.prototype members the evaluator allows for arbitrary access to objects' prototypes & invoking of Javascript functions. Oct 1, 2020
@astanciu
Copy link

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

oatkachenko added a commit to elsci-io/elsci-jexl that referenced this issue Jun 18, 2021
oatkachenko added a commit to elsci-io/elsci-jexl that referenced this issue Aug 25, 2022
…otypes or invoke Javascript functions"

This reverts commit 80fa1d5.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants