Open xiaofen9 opened 4 years ago
Thanks for the quick response. The following link points to the actual location of where the vulnerable code is: https://github.com/typestack/routing-controllers/blob/aae917bc093aa4e64f584f2660a490fda73fda5c/src/ActionParameterHandler.ts#L147
I've consulted the class-validator contributors and their suggestion is to use forbidUnknownValues
option when invoking validator. I think this is good enough to fix this bug.
https://github.com/typestack/class-validator/issues/438#issuecomment-544616992
EDIT a slightly better (?) monkeypatch that eliminates cause and not effect.
export default function fixValidation() {
const original = JSON.parse
JSON.parse = function (obj, reviver) {
return original.call(this, obj, (key, value) => {
if (key === '__proto__') {
return undefined
}
if (reviver) {
return reviver(key, value)
}
return value
})
}
}
Monkey-patch I use that somehow works, but breaks with complex cases
// @ts-nocheck
/* eslint-disable */
import { ActionParameterHandler } from 'routing-controllers/ActionParameterHandler'
import { BadRequestError } from 'routing-controllers'
import { ValidationExecutor } from 'class-validator/validation/ValidationExecutor'
export default function fixValidation() {
// top-level guys
let validateValue = ActionParameterHandler.prototype.validateValue
ActionParameterHandler.prototype.validateValue = function (value, paramMetadata) {
if (typeof value === 'object' && !(value instanceof paramMetadata.targetType)) {
throw new BadRequestError('Malformed request')
}
return validateValue.call(this, value, paramMetadata)
}
// nested guys
const execute = ValidationExecutor.prototype.execute
ValidationExecutor.prototype.execute = function (object, targetSchema, validationErrors) {
if (!this.validatorOptions) {
this.validatorOptions = {}
}
this.validatorOptions.forbidUnknownValues = true
return execute.call(this, object, targetSchema, validationErrors)
}
}
Is that still an issue on the latest version available?
I'm running a simple test, I'm missing something? https://gist.github.com/jkiyo/735c7f363d469d777be0d5767e9c9042
Versions:
"class-transformer": "0.3.1",
"class-validator": "0.12.2",
"routing-controllers": "^0.9.0",
Edit:
It looks like class-transformer
has this issue fixed on 0.3.1 by cleaning pollution from objects.
routing-controllers@^0.9.0
already requires the fixed version of class-transformer
.
Is this still relevant?
We found that the input validation in routing-controllers can be bypassed. With this vulnerability, attackers can launch SQL Injection, XSS attacks by injecting malicious inputs.
routing-controllers use class-validator to validate user-input. However, an attacker can corrupt a critical internal attribute used by class-validator (i.e., constructor) by injecting an additional attribute to the user-input. The corruption can be done because routing-controller uses the class-transformer to convert user-input to the validation class instance, and the conversion will also overwrite the previous internal attribute if it exists in the user-input.
Proof of Concept: Before corruption![2](https://user-images.githubusercontent.com/20917869/67150006-12ffcb00-f280-11e9-8a92-ce9b646a88bd.png)
After corruption![1](https://user-images.githubusercontent.com/20917869/67150007-14c98e80-f280-11e9-95a7-a5600e2d4eae.png)
This issue goes all the way down to the underlying lib (class-validator) used by routing-controller, and we have reported this issue to this lib. However, just to be safe, my suggestion is that routing-controller should also filter proto attribute before invoking class-validator since it is an internal attribute used by class-validator and should never appear in user-input.