-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Bump uglify version to fix vulnerability #1084
Conversation
Ugh... do you know if any of Handlebar's code is impacted by this? Will certainly merge this but leaving open for right now while we assess the impact that this might have. My hope is that it's minor as there aren't a whole lot of combined flags like the exploit mentions but I don't fully understand the exploit yet. |
A better source if you're having trouble understanding the vulnerability is here: https://zyan.scripts.mit.edu/blog/backdooring-js/ I looked into it, uglify is used in lib/precompiler.js. It seems to me that any code that passed to the precompiler with the -m or --min flags set is run through the vulnerable version of uglify and is therefore vulnerable. So, it's not a matter of the handlebars code itself being vulnerable, rather any client code that's using those options. |
Even though Handlebars is not effected by the vulnerability. Tools like grunt-nsp-shrinkwrap will give warnings until Uglify-JS is updated. And in my case it breaks our Jenkins build process (which it should since there might be a dangerous vulnerability, that should be fixed). |
@hacker112, Handlebars may not itself be vulnerable to backdooring, but it is affected by the vulnerability because any code that uses handlebars is now potentially a target. |
My read on this is that the code that is vulnerable is not written anywhere in Handlebars nor is it generated by our code generation logic. We have only one location that we combine not operations with non-boolean responses and that case doesn't minify to anything different that what is written. Merging this but since we have a number of breaking changes on the tree and the current release has some additional breaking changes that are still under review we can't cleanly roll a release right now. This release is close but I can't estimate when I'll get community feedback on the open pull requests to provide a better estimate on when this release might go out. Those who are cautious can minimize their own copy of Handlebars and perform their own minimization of the templates. Since the uglify code is not shipped to the client in any way and not utilized within node land unless you are using the precompiler, any perceived risk can easily be mitigated. Generally we recommend running your own compilation and minification regardless as the Handlebars CLI is provided as a connivence but is not intended for a production caliber release process. If someone is aware of a specific bit of our code, generated or written, that is vulnerable in our code please email me directly and I'll be glad to see what we we can do to fix it discretely and out of band if need be. |
Bump uglify version to avoid vulnerability flag.
Thank you. |
@kpdecker what are the chances of getting this into a 3.0.4 release? I'd love to not have false positives in my NSP alerts. |
Pretty much nil for the reasons outlined above. The tree is not in a releasable state and since this isn't an actual exploit it doesn't warrant an out of band release, particularly when other changes have been promised and we cherry-picking those changes would be a nightmare. |
You don't think an issue that breaks a lot of dependent builds warrants a patch release? I'm with @RCanine here. I completely understand it's a distraction and a pain (sincerely), but a lot of companies have a policy of no broken builds, which means this will force a drastic short-term action, like abandoning handlebars in favor of an alternative that doesn't leave builds broken. |
At a risk of sounding naive, it should be trivial to create a new branch from 3.0.3 tag and apply this change there. What am I missing? Why is it hard to make 3.0.3.1 release? |
I am thinking the same thing as @agladysh. Its one of the benefits with good versioning system that handles branches well like Git and Mercurial. |
If problem is really with Git (I suspect that it is not), I can help with creating a branch for the patch release. |
Released in 4.0.0 |
For more information see: https://nodesecurity.io/advisories/uglifyjs_incorrectly_handles_non-boolean_comparisons