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

security: fixed issue with class-validator dependency cve #214

Merged
merged 3 commits into from
Feb 8, 2023
Merged

security: fixed issue with class-validator dependency cve #214

merged 3 commits into from
Feb 8, 2023

Conversation

akatsukilevi
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Related issue linked using fixes #number
  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

Previous version is using a vulnerable version of class-validator that suffers from a SQL injection and Cross-site Scripting (GHSA-fj58-h2fr-3pp2). This vulnerability affects all versions below 0.14.0.

This PR seeks to fix it by updating it(together to @latipun7/releaserc and semantic-release that npm audit has showed to be fixable, and were bumped to the highest supported version that did not introduced breakage) to at least 0.14.0

What is the current behavior?

Behavior persists the same without changes, aside from the above mentioned CVE no-longer affecting this project.

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Tests have been ran multiple times locally, and it shows to not have changed any sort of behavior or inner-works of the library.
Would be glad if this could be double-checked before merged in

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@akatsukilevi
Copy link
Contributor Author

Okay, only now that I noticed, why did package-lock.json has so many changes???? I only updated a few packages using npm

@Nikaple
Copy link
Owner

Nikaple commented Feb 8, 2023

Seems that you are using npm@9 which generates a package-lock.json file with "lockFileVersion": 3, could you downgrade to npm@7~8 and use "lockFileVersion": 2 to keep it compatible with node@12 and node@14?

@akatsukilevi
Copy link
Contributor Author

This explains it, hold a bit, will downgrade on my side and re-generate the package-lock.json

@akatsukilevi
Copy link
Contributor Author

Wrong. Node. Version. Used... I set NVM to use Node 15 + NPM 7, yet my terminal just didn't picked it up and still used Node 18 + NPM 9

@akatsukilevi
Copy link
Contributor Author

Okay, this time it's properly fixed! Lock file version 2

$ node -v
v15.14.0
$ npm -v
7.7.6

Used Node 15 because it was the one listed on the changelog with support for NPM 7 that you mentioned. I also installed Node 12, and all tests pass just fine on it!

image

Only real issue is http-cache-semantics which can't be fixed due to @latipun7/releaserc depending on a specific version of semantics-release, yet don't think it will affect much aside from CI/CD.

$ npm audit
# npm audit report

http-cache-semantics  <4.1.1
Severity: high
http-cache-semantics vulnerable to Regular Expression Denial of Service - https://github.com/advisories/GHSA-rc47-6667-2j5j
fix available via `npm audit fix`
node_modules/npm/node_modules/http-cache-semantics

1 high severity vulnerability

To address all issues, run:
  npm audit fix

@Nikaple Nikaple merged commit b12063f into Nikaple:main Feb 8, 2023
@akatsukilevi akatsukilevi deleted the fix/class-validator branch February 8, 2023 17:13
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

2 participants