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

Class-validator vulnerabilities #1443

Closed
msmadja-dev opened this issue Dec 9, 2021 · 15 comments
Closed

Class-validator vulnerabilities #1443

msmadja-dev opened this issue Dec 9, 2021 · 15 comments
Labels
status: duplicate Issue is being tracked already in another issue. type: fix Issues describing a broken feature.

Comments

@msmadja-dev
Copy link

msmadja-dev commented Dec 9, 2021

Description

Recently found for class-validator critical vulnerability issue:
https://snyk.io/vuln/npm:class-validator.

Class-validator is a decorator-based property validation for classes.

Affected versions of this package are vulnerable to Improper Input Validation via bypassing the input validation in validate(), which can lead to cross-site scripting (XSS) or SQL injection. NOTE: There is an optional forbidUnknownValues parameter that can be used to reduce the risk of this bypass.

AR: Found vulnerability when running npm audit.
ER: No vulnerability found.

@msmadja-dev msmadja-dev added status: needs triage Issues which needs to be reproduced to be verified report. type: fix Issues describing a broken feature. labels Dec 9, 2021
@gokuldeep-kk
Copy link

Facing the same issue on Snyk Test - https://snyk.io/vuln/SNYK-JS-CLASSVALIDATOR-1730566

@rick-aguayo
Copy link

Any timeframe when this critical vulnerability will be assigned for fix?

@SandraShklyaeva
Copy link

+1 to have this fix

@razvanmihaimarin
Copy link

Any updates regarding this?

@narensgh
Copy link

When are we planning to take this up? This is also throwing vulnerabilities on AWS ECR after enabling image scanning.

@DnOberon
Copy link

Any updates on this? I really really really don't want to have to rip this library out and figure out a replacement.

@juansedo
Copy link

Duplicated of #1422.

Any update on this?

@debragail
Copy link

Hey this is a critical vulnerability can we get some eyes on this?

@DnOberon
Copy link

DnOberon commented May 1, 2022

honestly @juansedo @debragail I'm thinking this package is dead

@xujif
Copy link

xujif commented Oct 11, 2022 via email

@Shereef
Copy link

Shereef commented Nov 9, 2022

For the people asking if there's any update. I don't think there will be.

However, feel free to use this easy fix:

  1. npm uninstall class-validator
  2. npm install @nestjs/class-validator
  3. search your project for from 'class-validator' and replace it with from '@nestjs/class-validator'

P.S. This is what I did and it fits my project and it doesn't necessarily mean that you should do it or that it's right.

@braaar
Copy link
Member

braaar commented Nov 9, 2022

Duplicate of #1422

@braaar
Copy link
Member

braaar commented Nov 16, 2022

Closing this. Let's track this in #1422

@braaar braaar closed this as completed Nov 16, 2022
@braaar braaar added status: duplicate Issue is being tracked already in another issue. and removed status: needs triage Issues which needs to be reproduced to be verified report. labels Nov 16, 2022
@NoNameProvided
Copy link
Member

Hi all!

Sorry for the long overdue update, I would like to chime in to clarify a few things.

First of all, as mentioned before in other threads the reported issue is not a security vulnerability in the sense that you can defend against it by specifying the forbidUnknownValues: true option.

I still think this was opened by mistake and it's the same as if I would open a vulnerability report on NodeJS saying "if I turn off the server the NodeJS application crashes". The valid (and fixed) problem was in the class-transformer package that I fixed in a rather speedy manner in under 10 days from disclosure. (The fastest fix from the 3 affected packages.) That issue was properly reported privately to me with a pentest report explaining the attack vector.

A second problem is that for this vulnerability I was never provided a reproducible test case officially, saying: this is what failing and needs to be fixed. The closest to an official example is from the issue in this repository that is linked in the security reports: #438. Running that example code shows the issue is fixed for almost a year now.

Code snippet from #438
import { validate, IsInt, Length, IsEmail, IsFQDN, IsDate, Min, Max } from "class-validator"; 
import { plainToClass } from "class-transformer";

class Post {
  @Length(10, 20) 
  title: string; 
  
  @IsInt() 
  @Min(0) 
  @Max(10) 
  rating: number; 
  
  @IsEmail() 
  email: string; 
  
  @IsFQDN() 
  site: string; 
  
  @IsDate() 
  createDate: Date;
}

let userJson = JSON.parse('{"title":1233, "proto":{}}'); // a malformed input 
let users = plainToClass(Post, userJson);

validate(users, { forbidUnknownValues: true }).then(errors => { // errors is an array of validation errors 
  if (errors.length > 0) { 
    console.log("validation failed. errors: ", errors); 
  } else { 
    console.log("validation succeed"); 
  } 
});

This code fails with validation errors as expected.

So you may ask why the security advisory is still open? That's the million-dollar question, and the answer is that when you try to write to someone they will redirect to someone else who will redirect to someone else who will redirect to the first org. It's a circle and everybody says: "sorry I just source my data from someone else", I cannot do anything for you.

Another contributor tried to write to them, and I have tried to write to them. No success.

The mistake I made was that after a while I stopped trying. I knew the issue don't exist so I am using it without worrying, but I see how an open critical advisory is scary for others.

To sum up my plan for going at it again:

  • I will flip the default settings for forbidUnknownValues and make a release so there is literally zero ground to say this is a vulnerability
  • I will try to contact the SNYK and other organizations again to tell them the issue is not present
  • I will repeat step two until someone actually can update the security advisory

Also, it is worth noting that the other package under the NestJS org doesn't fix any issues. The security warning is not present there because it has a different name, not because the "problem is fixed".

PS: As @braaar mentioned, this is tracked in another issue now, please subscribe to that for future changes.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: duplicate Issue is being tracked already in another issue. type: fix Issues describing a broken feature.
Development

No branches or pull requests