-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Remove eval usage. Fixes CSP cases.
#1133
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
Conversation
023af73 to
6bb6cfa
Compare
src/visitors/globals.js
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is necessarily very reliable. What if someone declares window or self in the local scope of their module?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point. Replaced it with a more reliable (and succinct!) solution: 1fe753e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah but won't Function constructor have the same problem as eval with CSP?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're, right, my bad.
I don't know why I assumed that another variant of arbitrary code execution would fly. 🙃 Should have read W3C Working Draft for CSP…
About the case of window and self in a local scope – you're talking about a situation where there's a module that either:
- uses
globalobject, redefineswindow, and is used in a browser - uses
globalobject, redefinesself, and is used in a Web Worker
Do we have to choose between supporting those two cases and supporting CSP?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah... we may be able to get it working by moving that code to get the global object you originally added outside of each module and into the prelude where we can guarantee that nothing will mess with those variables. In there, the global object should be this (unless strict mode is enabled). Then we can attach it to module or something, and here assign var global = module.global, or maybe pass it into the module function. What do you think?
See this article for more info: https://www.contentful.com/blog/2017/01/17/the-global-object-in-javascript/
5c707c4 to
e35413b
Compare
|
@devongovett Moving acquisition of Btw, it would be a good idea to run some of the tests in an actual browser environment, maybe via Puppeteer. I'll have to look into that. Some actual CSP tests would be nice. |
Prevent violating Content Security Policy fixes parcel-bundler#335
The `global` object is aquired via `this` in a non-strict scope and then passed into each module.
|
There was one additional case to handle: when you redeclare |
|
I use latest Parcel 2 and I still see Package: "devDependencies": {
"electron": "^39.2.2",
"parcel": "^2.16.1",
"@parcel/transformer-vue": "^2.16.1"
},Output JS: // eval may also be disabled via CSP, so do a quick check.
var supportsSourceURL = false;
try {
(0, eval)('throw new Error("test"); //# sourceURL=test.js');
} catch (err) {
supportsSourceURL = err.stack.includes('test.js');
} |
Picks up on work done by @MBEIERL (closes #353).
Fixes #335.