#2355 Security issue in dependencies

[Anonymous]

Originally created by: noraj

hoek@2.16.3 is vulnerable to CVE-2018-3728

for node-sass the problem comes from requiring request@2.79.0 in the package.json

dependency tree is as follow for 4.8.3

node-sass@4.8.3
|-request@2.79.0
 |-hawk@3.1.3
  |-hoek@2.16.3

and is the same for 4.9.0:

node-sass@4.9.0
|-request@2.79.0
 |-hawk@3.1.3
  |-hoek@2.16.3

Fix

To fix this request@2.82.0 or superior is required.

Context

• NPM version (npm -v): 5.8.0
• Node version (node -v): v9.11.1

• Node Process (node -p process.versions):

  { http_parser: '2.8.0',
  node: '9.11.1',
  v8: '6.2.414.46-node.23',
  uv: '1.19.2',
  zlib: '1.2.11',
  ares: '1.13.0',
  modules: '59',
  nghttp2: '1.29.0',
  napi: '3',
  openssl: '1.0.2o',
  icu: '61.1',
  unicode: '10.0',
  cldr: '33.0',
  tz: '2018c' }
  - Node Platform (node -p process.platform): linux
  - Node architecture (node -p process.arch): x64
  - node-sass version (node -p "require('node-sass').info"):

  node-sass 4.9.0 (Wrapper) [JavaScript]
  libsass 3.5.4 (Sass Compiler) [C/C++]
  - npm node-sass versions (npm ls node-sass):

  ├─┬ gulp-sass@4.0.1
  │ └── node-sass@4.9.0 deduped
  └── node-sass@4.9.0

Related issues

request:

https://github.com/request/request/issues/2926
https://github.com/request/request/issues/2874

node-sass:

https://github.com/sass/node-sass/issues/2352
https://github.com/sass/node-sass/issues/2288
https://github.com/sass/node-sass/issues/2262
https://github.com/sass/node-sass/issues/2252
https://github.com/sass/node-sass/pull/2170
https://github.com/sass/node-sass/pull/2256

Problem

xzyfer in [#2352]

It cannot be fixed without break node < 4 support

I also see in [#2288] that the problem is solved in node-sass v5.

So this ticket need to stay opened until v5 is released. Please don't close it.

[Anonymous]

Originally posted by: samrispaud

i see the fix is on master :) is there a timeline on when the next major or minor release will be with this change?

[Anonymous]

Originally posted by: xzyfer

Subscribe to the v5 pr. I'm in holidays now so not for a couple weeks.

On Mon., 7 May 2018, 10:11 pm Sam Rispaud, notifications@github.com wrote:

i see the fix is on master :) is there a timeline on when the next major
or minor release will be with this change?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/sass/node-sass/issues/2355#issuecomment-387188864,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAjZWHnkAOwGZ6HRxWYeb6rXfVHkiwRkks5twKpTgaJpZM4Tq1z4
.

[Anonymous]

Originally posted by: noraj

@samrispaud

That's what I said ;-)

I also see in [#2288] that the problem is solved in node-sass v5.

So this ticket need to stay opened until v5 is released. Please don't close it.

[Anonymous]

Originally posted by: cloakedninjas

What about creating a 4.10.0 release that just bumps the request dependency ?

[Anonymous]

Originally posted by: xzyfer

Bumping the dependency would break support for Node < 4

On Wed., 9 May 2018, 12:41 pm Daniel Jackson, notifications@github.com
wrote:

What about creating a 4.10.0 release that just bumps the request
dependency ?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/sass/node-sass/issues/2355#issuecomment-387698434,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAjZWPrx7wQEUkTqYNHbDloFb4PBwGhAks5twsfagaJpZM4Tq1z4
.

[Anonymous]

Originally posted by: noraj

@cloakedninjas It's a wait and see for v5 release.

[Anonymous]

Originally posted by: chriseppstein

As I understand it, you can change the node engine version requirement and do a minor release. It doesn't violate semver for the people on the old node versions because the new versions of the package are excluded from that code's dependency version resolution.

[Anonymous]

Originally posted by: xzyfer

My understanding is that the engine property has no actual effect beyond
npm producing a warning. The install will still happen.

The way to resolve this error without breaking BC is to replace request all
together with something that's supports Node 0.10+

On Wed., 9 May 2018, 9:22 pm Chris Eppstein, notifications@github.com
wrote:

As I understand it, you can change the node engine version requirement and
do a minor release. It doesn't violate semver for the people on the old
node versions because the new versions of the package are excluded from
that code's dependency version resolution.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/sass/node-sass/issues/2355#issuecomment-387847684,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAjZWJUyGWTrunvxfUG5PIvAVtwZ4xNBks5tw0IQgaJpZM4Tq1z4
.

[Anonymous]

Originally posted by: ataylorme

Node 4 became end of life on May 1st (source). Can you provide some background on the decision to continue supporting it after end of life rather than pushing out a security fix? I'm not disagreeing with any decision but rather seek to understand it. For example, are there a large number of Node 4 users?

[Anonymous]

Originally posted by: noraj

Following this we should at least be under nodejs 6.x.

@ataylorme Personally I'm a archlinux user so it's nodejs 10.x for me :)

[Anonymous]

Originally posted by: florianbrinkmann

node-gyp, another dependency of node-sass, also uses the old version of the request library. I asked if they would update it, but the answer was no. Does that mean that the security issue will remain because both request dependencies will be loaded (the updated one from node-sass and the old one from node-gyp)?

[Anonymous]

Originally posted by: brlodi

node-gyp is targeting just "request": "2", which will match any version 2.x.x. It should use whatever version of request is pulled in by other dependencies (with deduplication) or the most recent 2.x.x release if nothing else in the tree requires it.

[Anonymous]

Originally posted by: chriseppstein

@xzyfer huh, I guess that's true now that engine-strict defaults to false and engineStrict is no longer supported in package.json. that's super irritating.

[Anonymous]

Originally posted by: florianbrinkmann

node-gyp is targeting just "request": "2", which will match any version 2.x.x. It should use whatever version of request is pulled in by other dependencies (with deduplication) or the most recent 2.x.x release if nothing else in the tree requires it.

Ah okay, did not understand that before (I got the info on the node-gyp repo, too), but now I got it, thanks! :)

[Anonymous]

Originally posted by: lsimone

so, is there a way to solve the issue github is warning me about?

[Anonymous]

Originally posted by: brlodi

@florianbrinkmann no worries, I was two sentences into a comment agreeing with you over on the node-gyp issue before my brain finished processing what a bare semver "2" meant

[Anonymous]

Originally posted by: johndatserakis

@lsimone current consensus I think is to wait for the v5 merge https://github.com/sass/node-sass/pull/2312 as @xzyfer mentioned. I'm waiting for the same thing.

[Anonymous]

Originally posted by: brlodi

@lsimone alternatively, if you need to fix it immediately and your project will support it, you can roll back to node-sass@4.7.0 exactly as mentioned in this comment on [#2288].

[Anonymous]

Originally posted by: ntwb

There is no version 4.7.0 published on npmjs.com, 4.6.9 and 4.7.1

https://www.npmjs.com/package/node-sass?activeTab=versions

There is a 4.7.0 tag here on GitHub though: https://github.com/sass/node-sass/tree/v4.7.0

[Anonymous]

Originally posted by: lysla

hey, still confirmed we waiting v5 for this fix? aside of the 4.7.0 downgrade as a temporary workaround

[Anonymous]

Originally posted by: generalov

Yarn users could use "resolutions" in package.json for temporary workaround.

 "resolutions": {
    "node-sass/**/request": "^2.82.0"
  }

https://yarnpkg.com/lang/en/docs/selective-version-resolutions/

[Anonymous]

Originally posted by: Fingel

Bumping the dependency would break support for Node < 4

In my opinion, fixing security vulnerabilities should be higher priority than supporting deprecated runtimes.
Now that Github sends out vulnerability reports, any project depending on node-sass has been looking pretty bad in the last 3 weeks.

[Anonymous]

Originally posted by: nschonni

non official response as an issue janitor on this project We appreciate that security checkers are flagging the dependency, and that concerns you. request is used to download the binaries for our package only, but because of the prototype pollution aspect that can still be a concern. We have fallback binary download options through https://github.com/sass/node-sass#binary-configuration-parameters, and our version of request shouldn't interfere with your production version that is pinned to a better version of request.
We are not going to break backwards compatibility in our v4, but we've already addressed this in the v5 branch. I only mop up issues, and @xzyfer is on a well deserved vacation, so chiming in that we need to address this isn't going to move this faster. I've tried to avoid ad hominem attacks, and I ask you all to do the same ❤️ ❤️ ❤️

[Anonymous]

Originally posted by: chase-moskal

why hello everybody, i am the workaround fairy

update node-sass in your package.json to a commit in the v5 branch

"node-sass": "git+https://github.com/sass/node-sass.git#a8005ad3e34b3310536888c299a186623a178f80",

now the security warnings are gone, and i'm in the future 👍

[Anonymous]

Originally posted by: guidobouman

Hey @xzyfer & @nschonni,

How do you suggest we install 4.7.0, when that version is not published to NPM? See @ntwb's comment. We are unable to use the suggested downgrade workaround as that version just does not exist on NPM.