search

airbnb / css

A mostly reasonable approach to CSS and Sass.
MIT License
6.95k stars 1.42k forks source link

stylelint-config-airbnb package's dep includes high priority vulnerability #84

Open ux-engineer opened 5 years ago

ux-engineer commented 5 years ago

Could you please update stylelint-config-airbnb package's dependencies, as these include high priority vulnerabilities?

npm audit

  High            Prototype Pollution                                           

  Package         lodash                                                        

  Patched in      >=4.17.11                                                     

  Dependency of   stylelint-config-airbnb [dev]                                 

  Path            stylelint-config-airbnb > editorconfig-tools > lodash         

  More info       https://npmjs.com/advisories/782                              

  High            Prototype Pollution                                           

  Package         lodash                                                        

  Patched in      >=4.17.12                                                     

  Dependency of   stylelint-config-airbnb [dev]                                 

  Path            stylelint-config-airbnb > editorconfig-tools > lodash         

  More info       https://npmjs.com/advisories/1065                             

  Moderate        Regular Expression Denial of Service                          

  Package         underscore.string                                             

  Patched in      >=3.3.5                                                       

  Dependency of   stylelint-config-airbnb [dev]                                 

  Path            stylelint-config-airbnb > editorconfig-tools > argparse >     
                  underscore.string                                             

  More info       https://npmjs.com/advisories/745
ljharb commented 5 years ago

There is zero risk here, since editorconfig-tools is a dev dep and only ran by devs of the package, so there’s nothing that needs doing.

It could be updated to use eclint, but that would only impact the < 3 developers who touch this project.

ux-engineer commented 5 years ago

@ljharb couldn't you just upgrade editorconfig-tools dependency's version to a higher one (if it has this lodash dep version upgraded to higher also)...?

A couple or so months ago many packages were giving this kind of high vuln alert because of an unpatched Lodash version dep...but those seem to have vanished now that packages have been updated.

When doing enterprise level application development with high security requirements, we are not happy to get notified of this kind of vulns even it's about only-dev-time dependency.

Neither does version tag "0.0.0" look trusthworthy considering the potential popularity of this package, so that's an another reason to do a dep upgrades update :)

ntwb commented 5 years ago

FYI: It's actually a dependency, not a devDependency, but thats moot IMHO.

@envision from what I've looked at this stylelint config is no longer maintained, so the "potential popularity" of this package is also moot.

If you like the rules currently used, then fork the package as it is MIT licensed and you can tweak it to your personal preferences.

I'd also suggest taking a look at either of these widely used stylelint configs: https://www.npmjs.com/package/stylelint-config-recommended https://www.npmjs.com/package/stylelint-config-standard

ljharb commented 5 years ago

Given that it's a runtime dep, then sure, we could switch it to eclint.

ashleyryan commented 5 years ago

npm audit is also reporting this moderate issue https://www.npmjs.com/advisories/745 from this package

jasonxxp commented 4 years ago

Given that it's a runtime dep, then sure, we could switch it to eclint.

look forward to switch it

oprudkyi commented 3 years ago

Hi, any updates on this ? Github Dependabot is spamming with alert about underscore.string version < 3.3.5, and only this package depends on it.

ljharb commented 3 years ago

Nope, no updates. You can tell GitHub to stop complaining about the warning in the meantime :-)

dzienisz commented 2 years ago

@ljharb I don't understand this approach, just update the deps or open merging requests to community.

ljharb commented 2 years ago

@dzienisz merging of what? Nobody’s sent a PR, which suggests it’s not important.

dzienisz commented 2 years ago

I don't know how to answer that. You have 10 PRs that are not merged. Do you want another one to have a bigger pile of PRs?

ljharb commented 2 years ago

Most of those are for translations, and two of them I just closed because they were unexplained changes. PR counts are irrelevant; even if there were 10,000 open PRs, you should still send a PR to a project if you want something prioritized.