fix: default settings allows arbitrary bypass vulnerability

[xiaofen9]

With this vulnerability, an attacker can bypass any security checks enforced by class-validator.

When class-validator is used to validate user-input, the attributes in the user-input object will be transformed into the validation class instance.
However, the transforming procedure will overwrite the internal attribute of validation class instance (e.g., constructor attribute) if the attacker injects an attribute with the same name into user-input. Once this internal attribute being overwritten, class-validator will be bypassed.

PoC

import {validate, validateOrReject, Contains, IsInt, Length, IsEmail, IsFQDN, IsDate, Min, Max} from "class-validator";
import {plainToClass} from "class-transformer";

class Post {

    @Length(10, 20)
    title: string;

    @Contains("hello")
    text: 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).then(errors => { // errors is an array of validation errors
    if (errors.length > 0) {
        console.log("validation failed. errors: ", errors);
    } else {
        console.log("validation succeed");
    }
});

Our suggestion is that class-validator should check the integrity of the constructor: if it is being corrupted, the validation should automatically fail.

[vlapo]

Transformation plain json object into class instance is not class-validator responsibility. I think this should be handled on class-transformer side.

[xiaofen9]

Thanks for the comment. we will report this to class-transformer since the transformer and validator are usually used together, which leads to vulnerable logics.

Even if class-transformer adds a patch for this issue, I think we might still want to add a simple check to the class-validator because we cannot guarantee that developers will only use class-transformer to transform those plain JSON object. For example, it is found that other methods (e.g., Object.assign() [1]) are used to transform user-input before validation. They will also invalidate the class-validator once attackers inject the same payload. From the perspective of these transformers, they just correctly transform all attributes.

If we can check whether the constructor of the validation class instance is empty or not, this bug will be fixed.

[1] https://github.com/MichalLytek/type-graphql/blob/44e12eefc97ff51cfa795502bb91ff8fcc9804b5/src/helpers/types.ts#L104

[vlapo]

I think for this purpose exists option forbidUnknownValues. It will return validation error in case class-validator dont know target class of validated object.

I know the name of option is missleading and also we do not have proper documentation.

Example:
https://stackblitz.com/edit/class-validator-boilerplate-gh518?file=index.ts

[xiaofen9]

Thanks for the comment. I think we should at least mention this option somewhere in readme right?

[vlapo]

Of course we need doc improvement in this area.

[xiaofen9]

Yes... because class-validator will be vulnerable to the mentioned attack in its default settings. We should let developers know this undocumented forbidUnknownValues option and use it when handling user-inputs or enforcing it as a default option when handling user-input...

[jmtelljohann]

Is there a reason why forbidUnknownValues shouldn't be default? It seems like an issue that class-validator has a security vulnerability in it's default state.

[NicoleG25]

@vlapo was this issue ever addressed?
Please note that CVE-2019-18413 was assigned.

[lalit0024]

@vlapo @xiaofen9 What's going on with this issue? Can you please provide an update.
Due to assigned CVE-2019-18413, corporate repo like nexus and other scanning tools prohibiting the library use being flagged as vulnerable and creating lots of issues for developers.