Skip to content
New issue

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

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

Already on GitHub? # to your account

eval() is used by argue.js which in return is used by gRPC #1096

Closed
StabbarN opened this issue Jan 21, 2018 · 3 comments
Closed

eval() is used by argue.js which in return is used by gRPC #1096

StabbarN opened this issue Jan 21, 2018 · 3 comments

Comments

@StabbarN
Copy link
Contributor

Eval is a security risk and should thus not be allowed but it is used by at least one package.

Adding app/index.js before the imports

window.eval = function() {
  throw new Error("Nope, eval not permitted");
};

makes decrediton fail to start.

More specifically:
app/node_modules/arguejs/argue.js:203 return eval( name );

gRPC imports argue.js at
https://github.com/grpc/grpc-node/blob/master/packages/grpc-native-core/package.json#L31

Some logs:

Error: Nope, eval not permitted at module.exports.window.eval (
http://localhost:3000/dist/bundle.js:68933:9) at Function.__.getType (
	decrediton/app/node_modules/arguejs/argue.js:203:12) at Function.__.belongs (
	decrediton/app/node_modules/arguejs/argue.js:242:19) at __ (
	decrediton/app/node_modules/arguejs/argue.js:29:13) at 
	ServiceClient.Client.makeUnaryRequest (decrediton/app/node_modules/grpc/src/client.js:507:14) at
	apply (decrediton/app/node_modules/lodash/lodash.js:499:17) at
	ServiceClient.wrapper [as version] (decrediton/app/node_modules/lodash/lodash.js:5356:16) at
	http://localhost:3000/dist/bundle.js:107164:27 at tryCallTwo (http://localhost:3000/dist/bundle.js:38536:5) at
	doResolve (http://localhost:3000/dist/bundle.js:38691:13)
@matheusd
Copy link
Member

Disclaimer: I haven't looked at the surrounding context of where gRPC uses argue to know if there are any actual exploit vectors (data coming from outside Decrediton going all the way to that eval call).

Obviously, having no eval() calls at all on the whole Decrediton stack would be great. But gRPC is a pretty fundamental dependency for Decrediton, so it will be hard to change it to something else.

So, before going down that road:

  • Can you provide a call stack that could trigger that eval with arbitrary third party data?
  • Is there a version of argue.js that doesn't use eval()?
  • Is the gRPC team aware of the presence of that eval() call in one of their depdencies? What is their position on this issue?

@StabbarN
Copy link
Contributor Author

I created an issue and they have already made a PR for fixing it, grpc/grpc-node#162 hurray :)

@matheusd
Copy link
Member

Good job! 👍

As soon as a new grpc release is made with the change we can update the dependency.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants