[Security] Bump websocket-extensions

[dawsbot]

We received a security alert for the firebase npm module which includes this downstream. Pre 1.0.4 versions of this package are at-risk of a Denial of service attack (dependable told us this):

[jcoglan]

Hi @dawsbot, websocket-extensions is also a package belonging to this organisation, and this security flaw was reported to, and fixed by, me. This PR looks similar to faye/websocket-driver-ruby#73 and I'm not sure why either is necessary -- applications should receive alerts for their full transitive dependency set via GitHub or other CVE alert channels, and indeed several of my personal repos have. What I'm unsure about is publishing new versions of every single library that depends, directly or otherwise on the affected package.

Just within this organisation, we have:

• websocket-extensions, the affected package
• permessage-deflate, which is a plugin for the above but has no formal dependency on it
• websocket-driver, which depends on websocket-extensions
• faye-websocket, which depends on websocket-driver
• faye, which depends on faye-websocket
• faye-redis, which is a plugin for faye

All of these modules have both Node.js and Ruby versions, and many other packages in those ecosystems depend directly or indirectly on some set of these. I don't believe it is or should be necessary for every package in the graph between the affected library and end-user applications to release a new version in order to make the patch "bubble up" manually.

We have mechanisms so that applications will get alerts if they are including a vulnerable package anywhere in their dependency tree, and they can update the package themselves or rely on dependabot to do it for them. If an application does not upgrade as a result of the security alerts we issued about websocket-extensions, it's not clear to me that they would upgrade any of the intervening packages that implicitly depend on it, whether those packages release security alerts for the new releases or not. Any application that is newly installing websocket-extensions will get the latest version, which is sufficient.

That being said, I'm open to doing the work of releasing new versions of all the above packages, if there's a convincing case that it will make it more likely that applications upgrade. One further question I have if we do that, is whether those releases should come with GitHub security alerts and/or CVEs, and if so what those alerts should say.

[jakubzitny]

Node.js libraries usually have a locking mechanism for locking dependency versions, either yarn.lock if you use Yarn or package-lock.json from NPM. I noticed that you don't have that in your repos @jcoglan, not here, nor in faye-websocket-node. This ensures that when a library is released, the (nested) dependencies have exact versions, not just versions that satisfy the semver constraints. If you added the locks to your libraries, you would be able to see that every fix/update in nested library should be included in all "parent" libraries. Otherwise, new users of your library would still be getting the old locked nested versions with vulnerability.

[jcoglan]

A piece of information that would really help me decide how to proceed with this is: @dawsbot @jakubzitny did you receive alerts on your repositories that are using this package, and were you able to upgrade it?

[dawsbot]

@jcoglan I use firebase in our application and I got this security alert. I updated thanks to dependabot, but if we didn't have dependabot, we'd be running around with a vulnerable version pinned to that vulnerable version "thanks" to our package-lock.json. Merging in the dependabot PR fully fixed this issue in our repo.

[jakubzitny]

We did receive alert in our internal CI via Snyk, however we don't use this library directly. We use it inside sockjs-client which uses faye-websocket which uses websocket-driver. For this case, where upgrade of sockjs-client is not yet available, I usually use yarn resolutions field, where I specify and upgraded version of nested dependency (websocket-extensions in this case) and that auto-generates the proper yarn.lock.

If I was a new "user" of sockjs-client though, I would only find out about this after my initial installation. I would not be able to get vulnerability-free version of it "out of the box". Now, I know sockjs-client is not your "problem", but we can only submit a bump PR for them when faye-websocket is OK, etc.

[derryl]

FWIW we also received an alert for our application (via Github). We're consuming websocket-extensions via the following path (versions are as reported in my lockfile):

• [email protected] (primary dependency)
• [email protected] + [email protected]
• [email protected]
• [email protected]
• [email protected]

Low priority for us, as webpack-dev-server is only executed for local development.

Still, I'm kinda surprised that we're not receiving the 0.1.4 patch version of websocket-extensions after reinstalling. Perhaps @jakubzitny is correct, that publishing your lockfiles would assist in this matter.

[jcoglan]

@dawsbot Relying on dependabot to do this is exactly what I expect end users to do, that's what it's there for.

@jakubzitny Likewise, using resolutions is the way I believe Yarn intends for people to handle this.

@derryl Your version of faye-websocket is current but your version of websocket-driver is very old (from 2016, the latest is 0.7.4). How are you attempting to upgrade your indirect dependencies and why is it not working?

[jakubzitny]

Yes, but resolutions field is a hack to override direct dependency that is not up-to-date with it's nested dependencies with fixed vulnerabilities. It is not a standard way. Standard way would be that I install the latest version of whatever and everything is "safe" without needing to add resolution, manually edit lock file or wait for a CI security check to fail.

[jcoglan]

After looking into this in more detail I realised my knowledge of Yarn is based on using it years ago, and on googling solutions to this problem that make it sound like upgrading indirect dependencies is hard, or not supported. Several issues on the Yarn git repo give that impression.

Its own documentation says this is exactly what resolutions is for, in the case that the version of a package you want is outside the range requested by its parent. However that's not the case here, because websocket-driver has an open-ended upper bound on its websocket-extensions dependency and so it should be completely possible to get the latest version of websocket-extensions by normal means.

I tried this out myself and forced Yarn to install webpack-dev-server with [email protected], by using this package.json:

{
  "dependencies": {
    "webpack-dev-server": "*"
  },
  "resolutions": {
    "websocket-extensions": "<0.1.4"
  }
}

Running yarn install with this file gives me [email protected] and records this in yarn.lock. If I then remove the above resolutions constraint and just run yarn upgrade, then websocket-extensions is upgraded to 0.1.4, both in node_modules and in yarn.lock, even though it's a very indirect dependency of webpack-dev-server.

Does this command not work for you? If not, are you able to determine why it is not giving you the latest version of this package, as it should?

[dawsbot]

If this repo relied on dependabot, you'd also get the same security alert I did and be prompted for a version upgrade. Can you introduce a lockfile to this repo, bump the vulnerable dep, and call it a day?

[jcoglan]

@dawsbot Libraries are not supposed to pin their dependencies, and npm does not include lockfiles when it builds packages, so adding a lockfile to this repository would have no effect.

The fact the people are asking me to do this indicates there's a problem I don't fully understand, and I'd rather get to the bottom of that before making hasty changes that will complicate the release process.

As far as I can tell, the only person in this thread that says they've been unable to install the latest websocket-extensions is @derryl. I would like to hear from them about what might be causing that before we make any decisions about a solution.

[tomato-gits]

I am also running into a similar issue. My security vulnerability is being reported by Github, and my project uses npm (6.14.4). 'Npm update' installs the latest version of websocket-extensions (0.1.4), but from what I can tell, the dependency listing from package.json in the websocket-driver is making its way into my project's package-lock.json file, which is triggering the Github warning despite the actual websocket-extensions package being up-to-date.

The text of Github's warning includes:

"We found a potential security vulnerability in one of your dependencies.
A dependency defined in server/package-lock.json has known security vulnerabilities and should be updated."

Here's the path by which I am consuming websocket-extensions:
....
├─┬ [email protected]
│ ├─┬ [email protected]
│ │ └── [email protected]
│ └── [email protected]
└─┬ [email protected]
└─┬ [email protected]
└── [email protected]

Since at least the beginning of version 6 of npm, when the package-lock.json file is created, it includes dependencies listed in "requires" object which seem to be lifted from the package.json files of each dependent project (see the end of the discussion here ).
Thus, every time I regenerate my package-lock.json file for my project (whenever npm update or install is run), while it does list the exact version of each package installed (in this case, 0.1.4 of the websocket-extensions package) it also includes a "requires" object listing the versions specified by the parent's package.json file (so it takes the rule ">=0.1.1" from websocket-driver's package.json and puts that in my package-lock.json).

The package-lock.json file ends up looking like this:

"websocket-driver": {
            "version": "0.6.5",
            "resolved": "https://registry.npmjs.org/websocket-driver/-/websocket-driver-0.6.5.tgz",
            "integrity": "...",
            "dev": true,
            "requires": {
                "websocket-extensions": ">=0.1.1"
            }
        },
        "websocket-extensions": {
            "version": "0.1.4",
            "resolved": "https://registry.npmjs.org/websocket-extensions/-/websocket-extensions-0.1.4.tgz",
            "integrity": "...",
            "dev": true
        },

When I push my project code to Github (including the package-lock.json file, which is the recommended practice, see here), their security scanner is looking at the package-lock.json file, sees the allowance for the vulnerable package as listed in the "requires" object, and puts up a vulnerability warning. It doesn't seem to matter that the actual package installed by the npm update (0.1.4) is the safe one, just that a potential vulnerable one is listed somewhere in that lockfile.

Manually changing the package-lock.json file (to change the websocket-driver's "requirements" listing in the package-lock.json file to >=0.1.4) makes the Github warning go away. However, this is not a permanent solution, since next time that I do an npm install/update of the project, it will update the package-lock.json file with the "requires" data lifted from your project's package.json, which will once again trigger the Github warning.

If I don't want to change anything about my project workflow (don't want to change the process of generating and committing the lockfile, don't want to fork your package, etc), there do seem to be some npm-specific solutions similar to the "resolutions" that yarn offers (for example here). However, the best suggested route still seems to be to file a PR such as this one.

I haven't figured out yet if adding a lockfile to your project would help in my specific case, but the change indicated in this PR would, since I think that would cause the info printed in my own project's lockfile to escape the Github vulnerability scan.

I hope this helps, but I am still pretty new to the world of dependency management, so let me know if I am mistaken about anything here!

[jcoglan]

@tomato-gits I've just tried out this workflow myself to see how GitHub handled it and I'm getting different results. First, I set up a project to depend on faye, which has websocket-extensions as an indirect dependency. I added "websocket-extensions": "0.1.3" to my dependencies to force an old version and then removed that line. The resulting commit is as you describe, re: the data for the websocket packages:

jcoglan/test-github-npm-sec@bd7d516#diff-32607347f8126e6534ebc7ebaec4853dR87-R101

When I pushed this commit, GitHub gave me an alert as you describe.

I then ran npm update --depth=100, the only effect of which was to change the resolved version of websocket-extensions. When I pushed this commit, the alert on GitHub disappeared. Note that the requires data for websocket-driver is unchanged.

The reason I'm trying to get to the bottom of this is that if people are struggling to get the right version of websocket-extensions, I don't see any reason that publishing new versions of its dependent packages would fix anything -- it just moves the problem to another package and all its dependents. The way this should be working is as I've just described: GitHub sends alerts to applications, which update their pinned versions, and the problem is sorted.

And just to reiterate: adding a lockfile to this repo would not help, because npm does not publish lockfiles when it builds packages. And since npm and yarn have different lockfile formats, the solution would only work for one of them anyway.

[jcoglan]

I also got a PR from dependabot, which it automatically closed once it noticed I updated the version. If you're seeing behaviour different from this, my instinct would be to take it up with GitHub.

[derryl]

@jcoglan Apologies for the extreme tardiness! I did not see these replies until now. I'll try running npm update tomorrow and let you know the results.

[slangberg]

Any progress?

[jcoglan]

@slangberg I'm not sure what action I need to take here, if any. Do you have an application in which you currently cannot upgrade websocket-extensions?

[slangberg]

Our group uses black duck in our CI pipelines, your package's latest version has been flagged for a critical security issue. We have had to force a downgrade as we are not allowed to proceed with the defect, due you know the nature of the issue and how could it could be resolved. If not prepare for a departure of users

[jcoglan]

@slangberg sorry, there's a few things I don't understand here, would you mind clarifying them for me:

• what is "black duck"?
• what exactly does "your package" refer to? what is the name and version of the package in question?
• which security issue is being flagged? do you know its CVE number?
• what does "force a downgrade" mean? what has been downgraded, which packages and versions aren't you allowed to run, and what are you running instead?
• do you know what is preventing you from installing packages in which the security bugs have been fixed?

[slangberg]

Black Duck provides a comprehensive software composition analysis (SCA) solution for managing security, quality, and license compliance risk that comes from the use of open source and third-party code in applications. The issue at the start of this thread is most likely the flaw that has been flagged but do not know exact reason. The package websocket-extensions v0.1.4 seems to have been a security flagged so I would guess any automated security stuff will flag that package version

I have tired #37 (comment) to see if this resolves the issue temporally
"resolutions": { "websocket-extensions": "<0.1.4" }

[jcoglan]

[email protected] does not have any known security flaws. All versions prior to 0.1.4 are affected by a ReDoS vulnerability as detailed in GHSA-g78m-2chm-r7qv. You should install version 0.1.4 and no earlier version.

[jcoglan]

Did everyone manage to upgrade websocket-extensions successfully, and can I close this issue?

[MoreSaltMoreLemon]

@jcoglan Thank-you for maintaining this package and several others.

I understand that dealing with numerous security warnings across multiple packages can be frustrating, as is the cascade of dependency updates that they can trigger.

But it would be greatly appreciated if you could raise this dependency to ">=0.1.4", as there are a number of upstream libraries which are depending upon it, and now that the security issue has been addressed, there is no reason to continue allowing users to install an insecure package version, nor require that they manually address the issue themselves.

[jcoglan]

@MoreSaltMoreLemon Something that would help here is if I understand what would lead to a user installing a vulnerable version today, and how bumping the dependency version in this package, and then this package's dependents, and those packages' dependents, and so on, would prevent that from occurring.

[MoreSaltMoreLemon]

@jcoglan Someone more knowledgeable than I might have a better take, but the issue as I see it is that, there is no version of websocket-driver which is actually safe and guaranteed not to use a pre 0.1.4 version of websocket-extensions.

Because of this, any upstream maintainers utilizing this library aren't able to bump their own requirements to a safe version, and the issue cascades all the way to the end developer who is now responsible for personally interceding in the versioning of a package 5+ levels down.

Personally I'm currently undergoing a dependency risk assessment which flagged this library, as well as a number of other libraries. But this library is unique in that it hasn't version bumped its dependencies in response to downstream risks, despite there being a secure version available, so I can't simply choose a newer version of my own direct dependency which doesn't have a pre 0.1.4 version.

As it stands, if I were to version bump a dependency which has this library further down the dependency tree, because this package's own dependencies are already set in the package-lock.json file, despite a newer version of websocket-extensions being available the locked version will be retained, forcing a manual intervention.

[jcoglan]

@MoreSaltMoreLemon What is the name of tool or process you refer to when you say "I'm currently undergoing a dependency risk assessment which flagged this library"? Who is performing this assessment and what tools and information are they using?

[MoreSaltMoreLemon]

@jcoglan The tool used for the assessment is WhiteSource, and that's about as much as I can answer at this time.

[jcoglan]

@MoreSaltMoreLemon Are you able to put me in contact with somebody from WhiteSource to discuss this?

[MoreSaltMoreLemon]

@jcoglan No, I'm afraid that I have no direct contact with someone at WhiteSource. WhiteSource will recognize the use of a pre-0.1.4 version of websocket-extensions as a security risk based on this report: CVE-2020-7662

[jcoglan]

@MoreSaltMoreLemon Are you able to say more precisely what you mean by "use" here? The thing a security audit should be concerned with is whether an application is loading and running insecure code. If this is the case, then I'm keen to understand how an application would come to be installing and running websocket-extensions@<0.1.4.

[jcoglan]

@MoreSaltMoreLemon One thing I forgot to ask earlier is, what's the complete chain of dependencies between your application and websocket-extensions, including their versions?

[emmygee1]

Yh