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

build(deps): ReDoS vulnerability from intermediate dependency #3125

Merged
merged 1 commit into from Jun 24, 2021

Conversation

ykolbin
Copy link
Contributor

@ykolbin ykolbin commented Jun 7, 2021

Hello folks,

CVE-2021-33623 describes ReDoS vulnerability from intermediate meow dependency, so I updated meow from 3.7.0 to 9.0.0.
Unfortunately I could not update to the latest version of meow (10.0.0), because meow code was migrated to ESM and node-sass requires node engine >= 12 according current package.json.

@emiwidknowit

This comment has been minimized.

@ykolbin
Copy link
Contributor Author

@ykolbin ykolbin commented Jun 8, 2021

There is an issue with Alpine release checks, but I've checked PRs nearby and it looks like common issue

@sass sass deleted a comment Jun 9, 2021
type: 'string',
},
includePath: {
type: 'string',
Copy link
Contributor

@nschonni nschonni Jun 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is fully equivalent with the below, since I think the API is expecting an array in all cases, which was why it coerced it if it wasn't. Maybe that isMultiple forces the same thing

Copy link
Contributor Author

@ykolbin ykolbin Jun 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

meow always returns array for isMultiple flags according documentation.
I checked behaviour and removed unnecessary code below as well: https://github.com/sass/node-sass/pull/3125/files/be85ce1818a68e45d4f40672fce5424a918bebd9#diff-66e4eb9929e494460303e4a5e5c4ea4252befaf983cc44bfea286987f0509ef9L285

@tsmartt

This comment has been minimized.

@sass sass deleted a comment from TannerS Jun 15, 2021
@sass sass deleted a comment from tvdijen Jun 15, 2021
@xzyfer xzyfer merged commit 30a52f7 into sass:master Jun 24, 2021
20 of 21 checks passed
@xzyfer
Copy link
Contributor

@xzyfer xzyfer commented Jun 24, 2021

Appreciate all your effort. This is released in 6.0.1.

@jimmywarting
Copy link
Contributor

@jimmywarting jimmywarting commented Jun 24, 2021

because meow code was migrated to ESM

Should also make the move to ESM - quite many are starting to do it right now

@xzyfer
Copy link
Contributor

@xzyfer xzyfer commented Jun 24, 2021

We're not open to adopting esm modules at this time. We want to minimise churn and limit releases to security patches and major compatibility issues.

@ljuroszekPerfectgym
Copy link

@ljuroszekPerfectgym ljuroszekPerfectgym commented Jun 24, 2021

@xzyfer the security first, nice that this PR is merged. I was waiting for it, like for gulp-sass pull

@xzyfer
Copy link
Contributor

@xzyfer xzyfer commented Jun 24, 2021

Thanks for the reminder @ljuroszekPerfectgym. I've released a 4.1.1 with the lodash update.

@psugihara-od
Copy link

@psugihara-od psugihara-od commented Jun 24, 2021

hell yeah, thanks!

@arjunadeltoso
Copy link

@arjunadeltoso arjunadeltoso commented Jul 6, 2021

Was this backported to 4.14.X version as well?

@xzyfer
Copy link
Contributor

@xzyfer xzyfer commented Jul 8, 2021

Looks like we can back port this to 4.x by updating to meow@7 without too much happy. I'll try to cut a release in the next 48hrs.

@justinwhall
Copy link

@justinwhall justinwhall commented Jul 13, 2021

Are there still plans to back port this to 4.x?

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

Successfully merging this pull request may close these issues.

10 participants