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 Advisory 961 - Denial of Service #2816

Closed
x87 opened this issue Jan 13, 2020 · 18 comments
Closed

NPM Advisory 961 - Denial of Service #2816

x87 opened this issue Jan 13, 2020 · 18 comments

Comments

@x87
Copy link

x87 commented Jan 13, 2020

yarn audit is failing due to the newly posted advisory https://www.npmjs.com/advisories/961.

Any plans on resolving this?

@saper
Copy link
Member

saper commented Jan 14, 2020

It would be nice if details were known...

@pratiksawant10
Copy link

pratiksawant10 commented Jan 14, 2020

Getting vulnerability issues for this package

npm audit

=== npm audit security report ===

Low Denial of Service

Package node-sass

Patched in No patch available

Dependency of node-sass [dev]

Path node-sass

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

@priorax
Copy link

priorax commented Jan 14, 2020

Since this was closed for not meeting the template, hopefully including this information will allow the conversation to be started:

  • NPM version 6.13.4
  • Node version v12.14.1
  • Node Process { http_parser: '2.8.0', node: '10.18.1', v8: '6.8.275.32-node.55', uv: '1.28.0', zlib: '1.2.11', brotli: '1.0.7', ares: '1.15.0', modules: '64', nghttp2: '1.39.2', napi: '5', openssl: '1.1.1d', icu: '64.2', unicode: '12.1', cldr: '35.1', tz: '2019c' }
  • Node Platform linux
  • Node architecture x64
  • node-sass versions
    node-sass 4.13.0 (Wrapper) [JavaScript]
    libsass 3.5.4 (Sass Compiler) [C/C++]
    npm node-sass versions:
├─┬ @angular-devkit/build-angular@0.13.9
│ └── node-sass@4.12.0 
└── node-sass@4.13.0 

When I run npm audit I get the same security warning as the other users above.
Hopefully that's all the information needed.

@Goonie
Copy link

Goonie commented Jan 14, 2020

From the npmjs page, for completion sake:

Denial of Service
node-sass, low severity

Overview
All versions of node-sass are vulnerable to Denial of Service (DoS). Crafted objects passed to the renderSync function may trigger C++ assertions in CustomImporterBridge::get_importer_entry and CustomImporterBridge::post_process_return_value that crash the Node process. This may allow attackers to crash the system's running Node process and lead to Denial of Service.

Remediation
No fix is currently available. Consider using an alternative package until a fix is made available.

@nschonni
Copy link
Contributor

nschonni commented Jan 14, 2020

I think what @saper is getting at is that this advisor doesn't link to any CVE, so although it might be fixed in later libsass releases, there is no way of knowing.
Reading the description, unless you're exposing node-sass to the web + letting them provide plug-in settings, this isn't exploitable, hence the "low severity"

@jmcvetta
Copy link

jmcvetta commented Jan 14, 2020

NPM website says this was reported by "Alexander Jordan", but with no link to the reporter. Quick googling suggests it may be @alexjordan, a security researcher with Oracle labs. Perhaps Alex will notice this mention and provide some more detail on this vulnerability.

@tonix-tuft
Copy link

tonix-tuft commented Jan 14, 2020

Same warning for me.

found 1 low severity vulnerability
  run `npm audit fix` to fix them, or `npm audit` for details

$ 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          │
└──────────────────────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Low           │ Denial of Service                                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ node-sass                                                    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ No patch available                                           │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ node-sass [dev]                                              │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ node-sass                                                    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/961                             │
└───────────────┴──────────────────────────────────────────────────────────────┘
found 1 low severity vulnerability in 15399 scanned packages
  1 vulnerability requires manual review. See the full report for details.

Will a patch be released? Thank you!

@saper
Copy link
Member

saper commented Jan 14, 2020

There will be no patch unless the details of the issue will be disclosed to node-sass or libsass team.

We don't know what is wrong. I have no idea what kind of funny vulnerability reporting process is this.

We need a Github issue with details how to reproduce this problem, as for any other issue.

@saper saper closed this as completed Jan 14, 2020
@saper saper pinned this issue Jan 14, 2020
cruddasj added a commit to InformedSolutions/JAQU-CAZ-National-Taxi-Register-UI that referenced this issue Jan 14, 2020
Update yarn audit severity flag in light of webpacker item. At present, no resolution is available from the maintainer (sass/node-sass#2816), therefore, the resolution approach is to allow informational and lows to return a standard exit code (rather than non-zero).
cruddasj added a commit to InformedSolutions/JAQU-CAZ-National-Taxi-Register-UI that referenced this issue Jan 14, 2020
Update yarn audit severity flag in light of webpacker item. At present, no resolution is available from the maintainer (sass/node-sass#2816), therefore, the resolution approach is to allow informational and lows to return a standard exit code (rather than non-zero).
@nschonni nschonni changed the title NPM Advisory - Denial of Service NPM Advisory 961 - Denial of Service Jan 14, 2020
@lenymo
Copy link

lenymo commented Jan 14, 2020

For anyone having CI-related difficulties with this vuln but who intends to keep using node-sass, I recommend npm-audit-resolver:
https://www.npmjs.com/package/npm-audit-resolver

@alexjordan
Copy link

alexjordan commented Jan 15, 2020

I'm the original reporter. The NPM security team should have disclosed the vulnerability (responsibly) to the maintainers and included its description and steps to reproduce.

@saper let me know whether I should post details here or on a private channel like email.

@nschonni
Copy link
Contributor

nschonni commented Jan 15, 2020

@alexjordan I tried the new GitHub Security Advisory thing, and invited you to the issue

@alexjordan
Copy link

alexjordan commented Jan 15, 2020

@alexjordan I tried the new GitHub Security Advisory thing, and invited you to the issue

Thanks. Information provided there.

@nschonni nschonni reopened this Jan 15, 2020
@nschonni
Copy link
Contributor

nschonni commented Jan 15, 2020

Thanks for the extra details @alexjordan
It was kind of what I guessed above. Unless you allow your users to pass their own imports, this isn't an issue for your projects.

@xzyfer
Copy link
Contributor

xzyfer commented Jan 15, 2020

Update: npm were informed of an possible issue June 2019 but failed to inform us before opening the security advisory 6 months later. We're currently working on a fix.

This was referenced Mar 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests