Skip to content

Fix CRITICAL MOCHA security vulnerabilities #23

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

Closed
wants to merge 1 commit into from

Conversation

james-pre
Copy link

This PR updates mocha in order to fix 3 critical level vulnerabilities. You can view them using npm audit on master.

Verified

This commit was signed with the committer’s verified signature. The key has expired and has been revoked.
james-pre James Prevett
@james-pre james-pre changed the title Fix *CRITICAL* security vulnerabilities Fix CRITICAL security vulnerabilities Dec 23, 2024
@james-pre
Copy link
Author

Can a maintainer please take a look at this? It's been almost a month.

@jonschlinkert @paulmillr

@jonschlinkert jonschlinkert changed the title Fix CRITICAL security vulnerabilities Fix CRITICAL MOCHA security vulnerabilities Jun 1, 2025
@jonschlinkert
Copy link
Member

Hi @james-pre, how can mocha have a vulnerability that impacts any production system?

@paulmillr
Copy link
Contributor

@james-pre before bothering maintainers via @-mentions for this shit:

  1. Investigate what’s ReDoS
  2. Investigate why would Mocha affect prod
  3. See if there disadvantages of upgrading mocha; such as: not supporting versions of nodejs which old version supported

@jonschlinkert
Copy link
Member

Agreed. I think it's worth keeping this here though, so other people can see these comments.

The main reason people create these issues is because they don't want to see the warnings in their terminal. This isn't how that problem should be solved. People need to create these issues on snyk. Stop paying snyk. Or tell snyk and companies like them to start compensating me and other developers for fixing bugs.

@james-pre
Copy link
Author

how can mocha have a vulnerability that impacts any production system?
Investigate why would Mocha affect prod

I never said it was on prod. Contributors still have to use computers though, and those computers are vulnerable.

Investigate what’s ReDoS

I did. It's ironic that there is a critical vulnerability involving regular expressions when this library involves regexes. Unfortunately, critical vulnerabilities tend to be... critical, which is the highest severity one can be.

See if there disadvantages of upgrading mocha; such as: not supporting versions of nodejs which old version supported

I already did. Mocha continues to support versions of Node.js that haven't been deprecated. In case you were wondering, deprecated means it is being phased out and shouldn't be used.

The main reason people create these issues is because they don't want to see the warnings in their terminal.

This is incorrect. Most developers don't want to have there life comprised by a stealer when they contribute to a product. Sure the vulnerabilities in this case aren't anything as bad as that, but becoming complacent opens up the possibility for more serve compromise.

@paulmillr
Copy link
Contributor

paulmillr commented Jun 2, 2025

I already did. Mocha continues to support versions of Node.js that haven't been deprecated. In case you were wondering, deprecated means it is being phased out and shouldn't be used

  1. https://github.com/mochajs/mocha/blob/00a895f6ad9c1e4c5500851d6ff875e8254a5e06/package.json#L485-L487
  2. https://github.com/mochajs/mocha/blob/4c558fb83ca5d7e260961b1ebfddcd377017a608/package.json#L42-L44

to-regex-range currently supports old node versions which are not supported by new mocha. Which will make it untestable. You want the package to bump minimum nodejs requirement? That would be a new major semver release. It is a bad idea to do this just because of shit vulnerability in devdep.

Guess what happens when new major ver of to-regex-range is released? Old Chokidar v2 and others who are using old to-regex-range — would NOT upgrade to it. The ReDos noise would still be kept in npm audit.

and those computers are vulnerable

Provide an exact exploit which makes computers vulnerable to stealers / trojans, using this ReDos as a part of exploit chain. You can't? Then it's low-severity.

becoming complacent opens up the possibility for more serve compromise

There are thousands of those useless ReDos shit vulnerabilities which do not matter at all. It's users who folow snyk notifications are being complacent - not maintainers of software. The notifications create useless noise.

There is high-signal stuff and there is low-signal stuff. This is low-signal. You can't steal shit or compromise user system using this. So it doesn't matter. ReDos basically means even in bad case, one (who is a CONTRIBUTOR TO to-regex-range; not end-user) can kill -9 the process.

@james-pre
Copy link
Author

to-regex-range currently supports old node versions which are not supported by new mocha

As you so vigorously pointed out, Mocha is a development dependency. No semver change needed.

It's users who folow snyk notifications are being complacent

I've seen you refer to Synk multiple times. I haven't and done intend to use Synk, so I have no idea what your talking about.

In any case, is it really too much work for you to review the 1 line development-only change and click "merge"?

Looking forward to your quick response.

@paulmillr
Copy link
Contributor

Mocha is a development dependency. No semver change needed

It would not run on old nodejs versions.

@paulmillr
Copy link
Contributor

In any case, is it really too much work for you to review the 1 line development-only change and click "merge"?

Merging or NOT merging useless pull requests like this sends a signal. In this case, by not merging it, we send a signal we do not consider this a vuln; or that we do not consider this as a fix; or both.

Run a test of new mocha version on node.js v8.0 (which to-regex-range supports) first. If it fails you're wasting our time.

@james-pre
Copy link
Author

I'm sorry for "bothering" you and "wasting" your time. I went out of my way to contribute to this project in a way I felt was meaningful and constructive. You met me with hostility and negative discussion. With all due respect, I'm appalled by your conduct.

I'm closing this PR since I don't feel like it has been productive or will be.

@james-pre james-pre closed this Jun 5, 2025
@james-pre james-pre deleted the deps branch June 5, 2025 01:22
@paulmillr
Copy link
Contributor

@james-pre we are tired of users harassing us for stuff which is not relevant to actual security of a system. This has been going on for years.

We are tired of solutions which don’t take into account our very specific requirements. You send pr which bumps minimum nodejs from 8 to 18 and makes tests non-runnable. The hell are you talking about?

Users see npm audit errors and they think something is vulnerable. When I see npm audit errors, I think that it’s a cancer which is slowly killing security supply chain by increasing severity of idiotic issues, such as ReDoS.

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

Successfully merging this pull request may close these issues.

None yet

3 participants