-
Notifications
You must be signed in to change notification settings - Fork 476
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
unsafe-eval security error in chrome extension #105
Comments
I see the offending piece of code is generated through minification (Closure Compiler or UglifyJS2). This is (most likely) something I cannot fix within URI.js' source. Maybe you'll have to use the unminified code instead… |
I see the same thing in the source as well: https://github.com/medialize/URI.js/blob/gh-pages/src/URI.js#L30 It could be changed to: var root = (function(f){ return (function() { return this; })(); })(); |
It's not that easy: var root1 = (function(f){ return f('return this')(); })(Function);
var root2 = (function(f){ return (function() { return this; })(); })();
root1 === root2; // is false, because root2 is undefined. Note: this is not the case if above code is run in the command line of your browser… |
Is the following also causing a SecurityError? var root = (function(f){ return new f('return this')(); })(Function); |
Still get the error. About your above code: var root1 = (function(f){ return f('return this')(); })(Function);
var root2 = (function(f){ return (function() { return this; })(); })();
root1 === root2; Did you run it from within an IIFE and got false? |
(function(){
var root1 = (function(f){ return f('return this')(); })(Function);
var root2 = (function(f){ return (function() { return this; })(); })();
root1 === root2; // is false
})(); if you run above code in the command line of your browser, you'll get |
This problem was introduced with #84 - maybe @jasonkarns has an Idea how to circumvent this SecurityError? |
Hmm. I don't recall Chrome throwing that error at the time I encountered that snippet. Perhaps noConflict should be backed out until there's a workaround? I've independently come to the conclusion that noConflict-style support is probably best handled by a micro-library, rather than natively within URI.js. Maybe it's just not worth it? |
To be fair, this SecurityError is thrown for Chrome Extensions - not the regular browser context. I don't mind noConflict(). I'd like to keep it. After all, Accessing the global context seems to be the problem - is there no other way to safely get to it? |
So the purpose of noConflict is to allow users, when in a browser context, to safely include URI.js while controlling the impact it has on the global namespace. This is only a concern for non-module users. URI.js uses the UMD module definition pattern such that AMD or CommonJS users are already in control of the namespace footprint of URI.js. The UMD pattern already has access to the root object at the time the factory function is invoked. A possible solution is to modify the factory function to accept the root object as the last parameter. Or alternatively, execute the factory function in the context of the root object, thus giving the factory function access to the same root object as the UMD wrapper. Both solutions would remove the need for the pseudo-eval entirely. |
I just pushed up two branches to illustrate the two alternatives mentioned above. Neither has been tested.
Quick and dirty implementation compared with master: jasonkarns/URI.js@medialize:master...root-factory-signature
Compared with master jasonkarns/URI.js@medialize:master...root-factory-context |
The issue is fixed in master and will be included in the next release |
The error seems to be coming because of the following code:
Error:
Reduced case chrome extension: http://dl.dropboxusercontent.com/u/42214070/urijs-security-issue.crx
The text was updated successfully, but these errors were encountered: