Skip to content
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

NPM Audit #1454

Closed
courtyenn opened this issue Jun 30, 2018 · 9 comments
Closed

NPM Audit #1454

courtyenn opened this issue Jun 30, 2018 · 9 comments

Comments

@courtyenn
Copy link

I am running Node Version 9.9.7 and I ran npm audit. It looks like there is a Cross-Site-Scripting issue.

high Cross-Site Scripting

Package handlebars

Dependency of mochawesome-screenshots

Path mochawesome-screenshots > handlebars

https://nodesecurity.io/advisories/61

@nknapp
Copy link
Collaborator

nknapp commented Jul 3, 2018

That's for versions < 4.0.0. Do you need a patch release of Handlebars 3.x that fixes the issue?

@mattolson
Copy link

Yes please. I just noticed that the fix was backported to 3.x but there is no 3.0.4 release that contains it. This would be very helpful for us!

@nknapp
Copy link
Collaborator

nknapp commented Dec 15, 2018

I have released 3.0.4 but then had problems with the travis build, so not all places were updated.
I have (hopefully) fixed those problems and released 3.0.5 right afterwards. So 3.0.5 and 3.0.4 are essentially identical, but 3.0.5 will hopefully reach the AWS cloud some time.

At the moment SauceLabs seems to be causing timeouts.

But it's in npm and RubyGems and it might work in bower (I had to do a manual fix there, so I'm not sure it will work).

nknapp referenced this issue Dec 15, 2018
There was a potential XSS exploit when using unquoted attributes that this should help reduce.

Fixes #1083
@nknapp nknapp closed this as completed Dec 15, 2018
@mattolson
Copy link

Thank you @nknapp !

@mattolson
Copy link

I wonder what the process is for getting https://www.cvedetails.com/cve/CVE-2015-8861/ updated...

@mattolson
Copy link

And then after the CVE is updated, https://github.com/nodejs/security-advisories/blob/master/ecosystem/handlebars/61.json will need to be updated as well.

@nknapp
Copy link
Collaborator

nknapp commented Dec 21, 2018

@mattolson @courtneynguyen some people have broken builds, because of the 3.0.5 release I made.
I should have expected this, and I'm considering to revert the commit and re-release. But I wanted your opinions as well. Is the XSS really an issue in your case, or do you just want to get rid of the audit warning? Please answer to #1489

@nknapp
Copy link
Collaborator

nknapp commented Jan 2, 2019

After talk to @wycats, we have decided to revert the fix (in 3.0.6), because it was in illegal change in a patch-release according to semver-logic.

The plan is to release a new minor version 3.1.0, in which the fix can be optionally activated. I'll open a separate issue for that.

In the meantime, people who require the fix must make sure to stick to version 3.0.5.

I'm sorry about the level of complexity of this issue, but this is an old project, with lots of users and compatibility is an important issue.

/cc @mattolson @courtneynguyen

@infolock
Copy link

infolock commented Mar 18, 2019

if you all are referencing the issue reported on snyk - i highly recommend taking a step back and thinking about what they are reporting against...

i'm not convinced that this is an issue at all with handlebars so much as it is an issue with the code by the developer(s) that potentially introduced this as an issue. (more information can be found in the comments in this PR - that is now closed after reading up more on the reported issue)

that said - please reconsider even attempting to patch this and reconsider pushing back on this one...

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

No branches or pull requests

4 participants