Skip to content

Commit

Permalink
Disable allErrors in default Ajv config
Browse files Browse the repository at this point in the history
Unfortunately `allErrors: true` is a DoS attack vector for certain
schemas.

See: ajv-validator/ajv@334071a
Ref: https://hackerone.com/reports/903521
  • Loading branch information
mcollina committed Jun 29, 2020
1 parent 7788b90 commit 627096f
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 23 deletions.
10 changes: 2 additions & 8 deletions docs/Validation-and-Serialization.md
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,6 @@ Fastify's [baseline ajv configuration](https://github.com/epoberezkin/ajv#option
removeAdditional: true, // remove additional properties
useDefaults: true, // replace missing properties and items with the values from corresponding default keyword
coerceTypes: true, // change data type of data to match type keyword
allErrors: true, // check for all errors
nullable: true // support keyword "nullable" from Open API 3 specification.
}
```
Expand All @@ -287,7 +286,6 @@ const ajv = new Ajv({
removeAdditional: true,
useDefaults: true,
coerceTypes: true,
allErrors: true,
nullable: true,
// any other options
// ...
Expand Down Expand Up @@ -522,7 +520,7 @@ Inline comments in the schema below describe how to configure it to show a diffe
```js
const fastify = Fastify({
ajv: {
customOptions: { allErrors: true, jsonPointers: true },
customOptions: { jsonPointers: true },
plugins: [
require('ajv-errors')
]
Expand Down Expand Up @@ -569,11 +567,7 @@ If you want to return localized error messages, take a look at [ajv-i18n](https:
```js
const localize = require('ajv-i18n')

const fastify = Fastify({
ajv: {
customOptions: { allErrors: true }
}
})
const fastify = Fastify()

const schema = {
body: {
Expand Down
4 changes: 3 additions & 1 deletion lib/schema-compilers.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ function ValidatorCompiler (externalSchemas, options, cache) {
coerceTypes: true,
useDefaults: true,
removeAdditional: true,
allErrors: true,
// Explicitly set allErrors to `false`.
// When set to `true`, a DoS attack is possible.
allErrors: false,
nullable: true
}, options.customOptions, { cache }))

Expand Down
2 changes: 1 addition & 1 deletion test/schema-serialization.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ test('Shared schema should be pass to serializer and validator ($ref to shared s
t.strictEqual(res.statusCode, 400)
t.deepEqual(res.json(), {
error: 'Bad Request',
message: 'body[0].location.email should match format "email", body[1].location.email should match format "email"',
message: 'body[0].location.email should match format "email"',
statusCode: 400
})
})
Expand Down
19 changes: 6 additions & 13 deletions test/validation-error-handling.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ test('should fail immediately with invalid payload', t => {
url: '/'
}, (err, res) => {
t.error(err)
t.deepEqual(JSON.parse(res.payload), {
t.deepEqual(res.json(), {
statusCode: 400,
error: 'Bad Request',
message: "body should have required property 'name', body should have required property 'work'"
message: "body should have required property 'name'"
})
t.strictEqual(res.statusCode, 400)
})
Expand Down Expand Up @@ -115,19 +115,12 @@ test('should be able to attach validation to request', t => {
}, (err, res) => {
t.error(err)

t.deepEqual(JSON.parse(res.payload), [{
t.deepEqual(res.json(), [{
keyword: 'required',
dataPath: '',
schemaPath: '#/required',
params: { missingProperty: 'name' },
message: 'should have required property \'name\''
},
{
keyword: 'required',
dataPath: '',
schemaPath: '#/required',
params: { missingProperty: 'work' },
message: 'should have required property \'work\''
}])
t.strictEqual(res.statusCode, 400)
})
Expand All @@ -154,7 +147,7 @@ test('should respect when attachValidation is explicitly set to false', t => {
t.deepEqual(JSON.parse(res.payload), {
statusCode: 400,
error: 'Bad Request',
message: "body should have required property 'name', body should have required property 'work'"
message: "body should have required property 'name'"
})
t.strictEqual(res.statusCode, 400)
})
Expand Down Expand Up @@ -184,7 +177,7 @@ test('Attached validation error should take precendence over setErrorHandler', t
url: '/'
}, (err, res) => {
t.error(err)
t.deepEqual(res.payload, "Attached: Error: body should have required property 'name', body should have required property 'work'")
t.deepEqual(res.payload, "Attached: Error: body should have required property 'name'")
t.strictEqual(res.statusCode, 400)
})
})
Expand Down Expand Up @@ -277,7 +270,7 @@ test('should return a defined output message parsing AJV errors', t => {
url: '/'
}, (err, res) => {
t.error(err)
t.strictEqual(res.payload, '{"statusCode":400,"error":"Bad Request","message":"body should have required property \'name\', body should have required property \'work\'"}')
t.strictEqual(res.payload, '{"statusCode":400,"error":"Bad Request","message":"body should have required property \'name\'"}')
})
})

Expand Down

0 comments on commit 627096f

Please sign in to comment.