indutny / asn1.js

ASN.1 Decoder/Encoder/DSL
MIT License
181 stars 64 forks source link

Remove eval #108

Closed DuBistKomisch closed 5 years ago

DuBistKomisch commented 5 years ago

vm.runInThisContext is essentially eval, and via browserify it is exactly eval

This was wrapped in a try-catch at least with #37, but the try still triggers a CSP violation/report.

Modern JS engines can infer the name from context, not just the define-time name, and with ES6 you can even do this properly without eval. Maybe we can get rid of this now?

https://stackoverflow.com/a/48899028 https://stackoverflow.com/a/9479081

noppa commented 5 years ago

This is also a adding lots of unneeded code for many people who use Webpack, because Webpack tries to automatically polyfill Node's Script module with a eval/iframe-based solution. Ironically, Webpack actually includes this library as an indirect dependency when someone tries to use the global crypto object and Webpack needs to polyfill that, too. This can be prevented manually with the right settings in Webpack config, but lots of people probably don't know about / aren't doing that.

indutny commented 5 years ago

Sorry, it should be fixed now!

DuBistKomisch commented 5 years ago

awesome thanks, now just need to wait a few years for it to flow up the transient dependencies :man_facepalming:

noppa commented 5 years ago

Thanks @indutny.

@DuBistKomisch if you happen to be using yarn instead of npm, you can use its resolutions option to force latest version of this.

package.json

"resolutions": {
    "webpack/**/asn1.js": "5.1.0"
}

did the trick for me. This might potentially break stuff, though, if there are breaking changes between the required version and the new one.

ChALkeR commented 4 years ago

parse-asn1 is still using asn1.js@4.10.1, which has named = require('vm').runInThisContext(.

Could perhaps parse-asn1 be bumped to use asn1.js@5, or this change go into asn1.js@4?

parse-asn1 looks highly popular (also required by webpack).

DuBistKomisch commented 4 years ago

btw, I ended up using yarn resolutions as well and it seems to be working, can't really track down why the major version got bumped anyway

+  "resolutions": {
+    "asn1.js": "^5.1.0"
+  },