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

fix: "an unknown value was passed to the validate function" #1873

Closed
Hatko opened this issue Dec 28, 2022 · 42 comments
Closed

fix: "an unknown value was passed to the validate function" #1873

Hatko opened this issue Dec 28, 2022 · 42 comments
Labels
status: needs triage Issues which needs to be reproduced to be verified report. type: fix Issues describing a broken feature.

Comments

@Hatko
Copy link

Hatko commented Dec 28, 2022

Description

After updating class-validator from 0.13.2 to 0.14.0, we get an error when consuming GraphQL API:

error: "Bad Request"
message: ["an unknown value was passed to the validate function"]
statusCode: 400

Minimal code-snippet showcasing the problem

import {
  Field,
  InputType,
  Int,
  ObjectType,
  ReturnTypeFuncValue,
} from '@nestjs/graphql'
import { IsOptional, IsPositive } from 'class-validator'

@InputType()
export class PaginationInfo {
  @Field(() => String, { nullable: true })
  before: string | undefined

  @Field(() => String, { nullable: true })
  after: string | undefined

  @IsOptional()
  @IsPositive()
  @Field(() => Int, { nullable: true })
  first: number | undefined

  @IsOptional()
  @IsPositive()
  @Field(() => Int, { nullable: true })
  last: number | undefined
}

Expected behavior

We expect to get a response, as with 0.13.2

Actual behavior

An error described above is thrown

@Hatko Hatko added status: needs triage Issues which needs to be reproduced to be verified report. type: fix Issues describing a broken feature. labels Dec 28, 2022
@luobogor
Copy link

+1

1 similar comment
@TEHbKA-dev
Copy link

+1

@Agreon

This comment was marked as outdated.

@Xalisse
Copy link

Xalisse commented Jan 12, 2023

as you're using nestjs, in your main.ts you can update your ValidationPipe creation as follow :
app.useGlobalPipes(new ValidationPipe({ forbidUnknownValues: false }))

@marlon-chagas
Copy link

+1

In NestJs, I got the same error but I was sending correct payload, without Unkown values, that makes no sense.

@pierre-aurele-martin
Copy link

I agree with @marlon-chagas - even with correct payload I had to disable forbidUnknownValues - something wrong here I think...

@nocive
Copy link

nocive commented Jan 17, 2023

please add a 👍 in the PR description instead of spamming with +1 comments :-)

@braaar
Copy link
Member

braaar commented Jan 24, 2023

I am not able to reproduce this.

Here's my attempt at a reproduction. I have commented the results of the console.logs:

import { IsOptional, IsPositive, validate } from "class-validator";

export class PaginationInfo {
  before: string | undefined;

  after: string | undefined;

  @IsOptional()
  @IsPositive()
  first: number | undefined;

  @IsOptional()
  @IsPositive()
  last: number | undefined;
}

const pInfo = new PaginationInfo();
pInfo.first = 2;
pInfo.last = 4;

const errors = await validate(pInfo);
console.log(errors); // []

pInfo.before = "before";
pInfo.after = "after";

const errors2 = await validate(pInfo);
console.log(errors2); // []

I see this is a GraphQL input object. Have you considered the possiblity that the __typename field could be the source of this? If you're not careful, that field can end up tagging along uninvited into your payload.

@patrickmichalina
Copy link

I am seeing this in a nestjs app as well after upgrading. Strange that it seems to work locally but fail with this error in higher remote environments 🤷🏻

@braaar
Copy link
Member

braaar commented Feb 1, 2023

I am seeing this in a nestjs app as well after upgrading. Strange that it seems to work locally but fail with this error in higher remote environments 🤷🏻

Have you double checked if your payload contains __typename or some other field that your validation class is not expecting?

@patrickmichalina
Copy link

Have you double checked if your payload contains __typename or some other field that your validation class is not expecting?

I suppose it is possible that production builds is attaching something. Need to check the output.

@TheGAFF
Copy link

TheGAFF commented Feb 2, 2023

We have several microservices using NestJS and this was happening to us for one particular endpoint. In our scenario, we had a property on our DTO that had no validation decorators on it. So for example:

export class MyDto {

myProperty: string;

}

became:

export class MyDto {

@IsString()
myProperty: string;

}

Which resolved the issue. Hope that helps

@janio02011998
Copy link

janio02011998 commented Feb 6, 2023

Hello guys, i found i guess the solution, in my DTO after add a class-validator this work very well.

wihout the validator in the DTO fields, i receve the error.

import { IsEmail, IsNotEmpty, MaxLength, MinLength } from 'class-validator';

export class CredentialsDto {
  @IsNotEmpty({
    message: 'Informe um endereço de email',
  })
  @IsEmail(
    {},
    {
      message: 'Informe um endereço de email válido',
    },
  )
  @MaxLength(200, {
    message: 'O endereço de email deve ter menos de 200 caracteres',
  })
  email: string;

  @MinLength(6, {
    message: 'A senha deve ter no mínimo 6 caracteres',
  })
  password: string;
}

@lays147
Copy link

lays147 commented Feb 9, 2023

Why the value of forbidUnknownValues changed between 0.13 and 0.14 that caused a breaking change for everyone is in a minor version and not in a major?

@destlaver
Copy link

how is possibile this issue is not fixed yet, it basically breaks the library

@gustawdaniel
Copy link

gustawdaniel commented Feb 21, 2023

Yes. It is total mess, and solutions are available only for NestJS.

Downgrade to:

    "class-validator": "^0.13.2",

fix problem.

@braaar
Copy link
Member

braaar commented Feb 24, 2023

Why the value of forbidUnknownValues changed between 0.13 and 0.14 that caused a breaking change for everyone is in a minor version and not in a major?

See Semantic Versioning's specification for version zero:

Major version zero (0.y.z) is for initial development. Anything MAY change at any time. The public API SHOULD NOT be considered stable.

On top of that I believe there is an unwritten convention for version zero releases to use minor version changes (0.13 -> 0.14) to reflect breaking changes, and patch versions (0.13.1 -> 0.13.2) for non-breaking changes. This gives users some predictability.

@Hatko – does the issue still persist even if your validation classes are set up to respect forbidUnknownValues?

@aangindra
Copy link

In my case, my DTO doesn't have "isOptional" from the class-validator; when I'm using the 0.13.2 version, I'm not facing an error even though I don't pass some params, but after upgrading to the 0.14.0 version, I got "an unknown value was passed to the validate function" error. So my simple solution is to add "isOptional" in my DTO (nestjs)

@eneilson
Copy link

In my case, my class property has a nested validation. Typing with class transformer @Type decorator resolved.

@ValidateNested() 
@ApiProperty() 
@Type(() => InstanceConfig) // here 
instances: Array<InstanceConfig>;

@okovpashko
Copy link

@eneilson thank you for pointing the way to dig. Your suggestion fixes the issue and seems like it's because by default class-validator doesn't transform nested objects to class instances.

In my case, the fix was a bit more complicated because I have a field containing a JSON string that I parse to an object using another decorator based on Transform from class-transformer. Adding the Type decorator did nothing because it seems to conflict with Transform (what it actually uses under the hood).

The simplest way to debug if the nested objects were parsed correctly is to log the parent DTO to the console and examine the value of the nested field. If it's just a plain object then class-validator will throw the "unknown value" error. If there's something like foo: FooClass { foo: 'bar' } then everything should work as expected.

@gaoljie
Copy link

gaoljie commented Feb 28, 2023

seems updating @nestjs/common to 9.3.9 will fix the problem

@ashrafatef
Copy link

what about non-NestJs users, it still makes no sense to set forbidUnknownValues: false, is there a solution for that? in the 0.13.x we could send context in the validationOptions at least can we have this back?

@KyleBrown-804
Copy link

@eneilson Thanks that was my issue as well, I just needed to include @Type in my Dto which I much prefer to just allowing unknown values.

yort-feng pushed a commit to apitable/apitable that referenced this issue Apr 28, 2023
Due to [a deserialization
issue](typestack/class-validator#1873) caused
by `class-validator` `0.14.0`, disable `forbidUnknownValues` for a
short-term solution.
@codeninja
Copy link

codeninja commented May 9, 2023

I am using Apollo and [email protected] and [email protected] to serve my graphql resolvers. I found locking my version of class-validator to [email protected] resolved this error for me.

@israelboudoux
Copy link

israelboudoux commented May 30, 2023

I'm having the same error when using version 0.14.0, but not when moving to 0.13.2, however this version has a critical severity vulnerability, any idea when a patched version will be released?

In my case I'm using NestJS with class-validator and when the backend tries to validate the object an error with the message an unknown value was passed to the validate function is being raised.

Thanks!

@chtpl
Copy link

chtpl commented May 31, 2023

Also make sure that if you create your DTO in the test with the ... operator based on another object, you are not providing the correct instance anymore.

In my case, I had tried
const deliveryAddress = {...invoiceAddress};

Instead I had to use
const deliveryAddress = Object.assign(new PostalAddressDto(), invoiceAddress);

@braaar
Copy link
Member

braaar commented May 31, 2023

@israelboudoux

any idea when a patched version will be released?

I have still not seen a reproduction of a supposed bug with forbidUnknownValues, and the gripes people have in this thread are largely caused by a lack of understanding of semantic versioning of major version 0, as I explained previously in this issue.

Until a bug can be reproduced, there is nothing to patch. If no reproduction is brought forth I am considering closing this issue, as letting it stay up is perpetuating a misconception (that the breaking change is a bug).

@israelboudoux
Copy link

@israelboudoux

any idea when a patched version will be released?

I have still not seen a reproduction of a supposed bug with forbidUnknownValues, and the gripes people have in this thread are largely caused by a lack of understanding of semantic versioning of major version 0, as I explained previously in this issue.

Until a bug can be reproduced, there is nothing to patch. If no reproduction is brought forth I am considering closing this issue, as letting it stay up is perpetuating a misconception (that the breaking change is a bug).

It doesn't make sense to close the issue when there are plenty of people having this issue.

I understand the semantic version but I didn't know that increasing a version would break something that was working correctly. It seems you're stating that this breaking change was on purpose, if so, could you please point out which one is it and how to address it?

I'll try to create a sample of an app emulating this issue and I'll share it later here.

Thanks!

@braaar
Copy link
Member

braaar commented Jun 1, 2023

I understand the semantic version but I didn't know that increasing a version would break something that was working correctly. It seems you're stating that this breaking change was on purpose, if so, could you please point out which one is it and how to address it?

Minor version increases within major version 0 (0.13.X -> 0.14.0) should not be considered non-breaking. I have linked to the relevant description of this in the SemVer spec previously in this thread. I believe it is common practice during version 0 that projects will use minor version bumps (0.13.X -> 0.14.0) for breaking changes and patch version bumps for non-breaking changers (0.13.0 -> 0.13.1).

This breaking change was made on purpose to fix a security vulnerability, see the release notes for a description of this.

I'll try to create a sample of an app emulating this issue and I'll share it later here.

That would be great, thanks!

@israelboudoux
Copy link

Hey @braaar,

Analyzing this issue in my application and it happens that when an endpoint receives a payload using the @Body decorator (nestjs), the output for param.constructor.name is Object for the object that should be validated and the validation fails. If I instantiate the class it using its constructor, the validation works normally (the output for ``param.constructor.nameisMyObject` for example).

Do you think turning forbidUnknownValues false would enable any vulnerabilities?

Thanks!

klerick added a commit to klerick/nestjs-json-api that referenced this issue Jun 12, 2023
1. Fix correct ajv schema for enum type of type orm
2. Add forbidUnknownValues for class-validator(typestack/class-validator#1873)
@braaar
Copy link
Member

braaar commented Jun 21, 2023

Hey @braaar,

Analyzing this issue in my application and it happens that when an endpoint receives a payload using the @Body decorator (nestjs), the output for param.constructor.name is Object for the object that should be validated and the validation fails. If I instantiate the class it using its constructor, the validation works normally (the output for ``param.constructor.nameisMyObject` for example).

Do you think turning forbidUnknownValues false would enable any vulnerabilities?

Thanks!

Nice find!

I'm no security expert, but the implication of allowing unknown values is that attackers could attach some sneaky extra stuff into an otherwise valid payload. Depending on the architecture of your application this could potentially allow them to give themselves heightened access or just simply corrupt the integrity of your database. Document databases like MongoDB and Firestore will typically allow you to save anything in there since there's no strict schema to adhere to.

const errors = await validate(input);
if(errors.length == 0) {
  await saveUserToDatabase(input) 
}

Of course, it's entirely possible that your application discards any extra fields from the input and I don't see a security issue in that case:, but in a way it's a bit scary to have to rely on future developers to remember this quirk when designing the code and it is arguably quite tedious:

const errors = await validate(input);
if(errors.length == 0) {
  await saveUserToDatabase({
    name: input.name,
    role: input.role
  }) 
}

@braaar
Copy link
Member

braaar commented Jun 21, 2023

Similarly, if you are doing some mapping from the input types onto another type after validation, you will have to be careful never to use the spread operator:

// vulnerable
const user = {
  name: input.name, 
  type: input.hasSubscription ? 'subscriber' : 'regular', 
  ...input // this could contain anything!
}
await saveUserToDatabase(user);

// safe
const user = {
  name: input.name, 
  type: input.hasSubscription ? 'subscriber' : 'regular', 
  role: input.role
}
await saveUserToDatabase(user);

I think the preferable option is to forbid unknown values since you will know that the shape of the object is what you think it is after validation.

@Mut1aq
Copy link

Mut1aq commented Jun 27, 2023

I was facing this issue too, on NestJS, here is what was wrong
First of all for easier debugging use this
app.useGlobalPipes( new ValidationPipe({ exceptionFactory: (e) => console.log(e), whitelist: true, transform: true, forbidUnknownValues: true, validateCustomDecorators: true, disableErrorMessages: isProduction, forbidNonWhitelisted: true, transformOptions: { enableImplicitConversion: true, }, }), );
And I would attach a debugger to figure out more info about the issue.

My issue was using a custom MongoDB ID pipe to transform params, here is the implementation

`
import {
PipeTransform,
Injectable,
ArgumentMetadata,
HttpException,
HttpStatus,
} from '@nestjs/common';
import { Types } from 'mongoose';

@Injectable()
export class MongoDBIDPipe implements PipeTransform {
transform(mongoDBID: string, _: ArgumentMetadata) {
if (!Types.ObjectId.isValid(mongoDBID)) {
throw new HttpException(
validation.invalidMongoDBID,
HttpStatus.NOT_ACCEPTABLE,
);
}
return mongoDBID;
}
}

`

this returns a string because that's how it arrives at the controller handler, but I wrote the type as Types.ObjectId, like this

`
@userid(new ValidationPipe({ validateCustomDecorators: true }))
userID: string,

@Query() filterQueryDto: FilterQueryDto,

@Param('messagesContainerID', new MongoDBIDPipe())
messagesContainerID: Types.ObjectId,

`
and that was causing the error to be thrown but I still didn't figure out how it relates to the DTO

@yuvalbl
Copy link

yuvalbl commented Jul 23, 2023

seems updating @nestjs/common to 9.3.9 will fix the problem

👏🏼 🎉
Thank you for that, saved me lots of time!

@tsaqifrazin1
Copy link

tsaqifrazin1 commented Aug 17, 2023

I have a similar problem when uploading files in Nestjs and get the same errors

when I downgrade the version of the class validator from 0.14.0 -> 0.13.2, the problem was solved. I'm using Nestjs version 9.x.x

@jasperboyd
Copy link

jasperboyd commented Aug 18, 2023

I was in a similar situation as many here upgrading this package in a nestJS app due to the security vulnerability. Would be great if they could just patch 0.13.x (show me the vulnerability fix and I'll make a PR).

For me part of the problem was in the internal handling of the TypeOrmCrudController. I didn't dig too far, in my case a PATCH update request was failing because not all properties that are passed on create/update were included. By manually defining a PATCH controller method and DTO to match the problem was fixed.

Hope you find this helpful let me know if I can clarify.

@sujitkv92
Copy link

Hi, I am getting the same error while I am using "class-validator": "^0.12.2", But just wonder this is working fine on local env. but getting below error on production env.
validation failed. errors: { undefined: 'An unknown value was passed to the validate function' }

@c4nzin
Copy link

c4nzin commented Nov 5, 2023

as you're using nestjs, in your main.ts you can update your ValidationPipe creation as follow : app.useGlobalPipes(new ValidationPipe({ forbidUnknownValues: false }))

Thank you mate, you saved me

@braaar
Copy link
Member

braaar commented Nov 17, 2023

Quoting myself from earlier in this issue:

I have still not seen a reproduction of a supposed bug with forbidUnknownValues, and the gripes people have in this thread are largely caused by a lack of understanding of semantic versioning of major version 0, as I explained previously in this issue.

Until a bug can be reproduced, there is nothing to patch. If no reproduction is brought forth I am considering closing this issue, as letting it stay up is perpetuating a misconception (that the breaking change is a bug).

I suspect this issue has gotten enough traction that people seeking guidance on how to make the breaking change in 0.13 work with NestJS will find it and see the tips and tricks people have posted in here. I will be closing this issue.

@braaar braaar closed this as not planned Won't fix, can't repro, duplicate, stale Nov 17, 2023
@ernanvr
Copy link

ernanvr commented Nov 25, 2023

I had similar issue. I fixed it changing param type from Types.ObjectId to string.
I am using mongoose + Nestjs

@ryanzhouff
Copy link

We have several microservices using NestJS and this was happening to us for one particular endpoint. In our scenario, we had a property on our DTO that had no validation decorators on it. So for example:

export class MyDto {

myProperty: string;

}

became:

export class MyDto {

@IsString()
myProperty: string;

}

Which resolved the issue. Hope that helps

Thanks, it helped!

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 Jan 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: needs triage Issues which needs to be reproduced to be verified report. type: fix Issues describing a broken feature.
Development

No branches or pull requests