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

sub dependency is vulnerable #59

Closed
garryyao opened this issue Mar 23, 2019 · 7 comments
Closed

sub dependency is vulnerable #59

garryyao opened this issue Mar 23, 2019 · 7 comments

Comments

@garryyao
Copy link

Hi

You might want to upgrade svgo for the following flaw:

> $ npm audit                                                                                               ⬡ 10.13.0 [±master ●]
                                                                                
                       === npm audit security report ===                        
                                                                                
┌──────────────────────────────────────────────────────────────────────────────┐
│                                Manual Review                                 │
│            Some vulnerabilities require your attention to resolve            │
│                                                                              │
│         Visit https://go.npm.me/audit-guide for additional guidance          │
└──────────────────────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Moderate      │ Denial of Service                                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ js-yaml                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=3.13.0                                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ babel-plugin-inline-react-svg [dev]                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ babel-plugin-inline-react-svg > svgo > js-yaml               │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/788                       │
└───────────────┴──────────────────────────────────────────────────────────────┘
@ljharb
Copy link
Collaborator

ljharb commented Mar 23, 2019

This isn’t actually a vulnerability for this package (npm audit has lots of false positives).

See #35 for why upgrading is prohibitively difficult.

@ljharb ljharb closed this as completed Mar 23, 2019
@cdeutsch
Copy link

A dependency of a dependency isn't your fault, but it's still your problem.

Why would this be considered a false positive?

I don't think this should be closed.

@ljharb
Copy link
Collaborator

ljharb commented Jul 23, 2019

@cdeutsch it's a false positive because the warning is for a DOS attack - which isn't possible in this case because you'd be attacking yourself. That's no more an attack than typing while (true) {} into your code.

@amitnovick
Copy link

Hi there!

Was going to use this package for a project but noticed the following 2 vulnerabilities appeared through npm's audit:

╰─$ npm audit
                                                                                
                       === npm audit security report ===                        
                                                                                
┌──────────────────────────────────────────────────────────────────────────────┐
│                                Manual Review                                 │
│            Some vulnerabilities require your attention to resolve            │
│                                                                              │
│         Visit https://go.npm.me/audit-guide for additional guidance          │
└──────────────────────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Moderate      │ Denial of Service                                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ js-yaml                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=3.13.0                                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ babel-plugin-inline-react-svg [dev]                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ babel-plugin-inline-react-svg > svgo > js-yaml               │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/788                       │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ High          │ Code Injection                                               │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ js-yaml                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=3.13.1                                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ babel-plugin-inline-react-svg [dev]                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ babel-plugin-inline-react-svg > svgo > js-yaml               │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/813                       │
└───────────────┴──────────────────────────────────────────────────────────────┘

The first vulnerability seems identical to OP's, but the second one hasn't been documented so far.

@ljharb
Copy link
Collaborator

ljharb commented Sep 1, 2019

In both cases, they’re false positives since you’ll be transforming your own code.

@amitnovick
Copy link

In both cases, they’re false positives since you’ll be transforming your own code.

Thanks for replying! much appreciated.

That said I still don't quite understand what you're saying.

This is a Babel plugin, so it leverages Babel's AST for transforming our code to some desirable target (in this case a React component with inlined <svg> element). Now I don't know how js-yaml is used in this plugin's code, but isn't it possible for js-yaml to inject some malicious code into our SVG component?

@ljharb
Copy link
Collaborator

ljharb commented Sep 1, 2019

No, that’s not what the audit report is about - it’s about the svg being transformed being able to inject malicious code. Since you’re transforming your own svgs, all you have to do is not put malicious code in them :-)

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