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

Protect against prototype poisoning #1427

Merged
merged 2 commits into from Feb 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 13 additions & 0 deletions docs/Server.md
Expand Up @@ -70,6 +70,19 @@ Defines the maximum payload, in bytes, the server is allowed to accept.

+ Default: `1048576` (1MiB)

<a name="factory-on-proto-poisoning"></a>
### `onProtoPoisoning`

Defines what action the framework must take when parsing a JSON object
with `__proto__`. This functionality is provided by
[bourne](https://github.com/hapijs/bourne).
See https://hueniverse.com/a-tale-of-prototype-poisoning-2610fa170061
for more details about prototype poisoning attacks.

Possible values are `'error'`, `'remove'` and `'ignore'`.

+ Default: `'error'`

<a name="factory-logger"></a>
### `logger`

Expand Down
1 change: 1 addition & 0 deletions fastify.d.ts
Expand Up @@ -178,6 +178,7 @@ declare namespace fastify {
interface ServerOptions {
ignoreTrailingSlash?: boolean,
bodyLimit?: number,
onProtoPoisoning?: 'error' | 'remove' | 'ignore',
logger?: any,
trustProxy?: string | number | boolean | Array<string> | TrustProxyFunction,
maxParamLength?: number,
Expand Down
2 changes: 1 addition & 1 deletion fastify.js
Expand Up @@ -208,7 +208,7 @@ function build (options) {
// custom parsers
fastify.addContentTypeParser = addContentTypeParser
fastify.hasContentTypeParser = hasContentTypeParser
fastify[kContentTypeParser] = new ContentTypeParser(fastify[kBodyLimit])
fastify[kContentTypeParser] = new ContentTypeParser(fastify[kBodyLimit], options.onProtoPoisoning)

fastify.setSchemaCompiler = setSchemaCompiler
fastify.setSchemaCompiler(buildSchemaCompiler())
Expand Down
35 changes: 22 additions & 13 deletions lib/contentTypeParser.js
@@ -1,6 +1,10 @@
'use strict'

const lru = require('tiny-lru')
const Bourne = require('bourne')
const {
kDefaultJsonParse
} = require('./symbols')
const {
codes: {
FST_ERR_CTP_INVALID_TYPE,
Expand All @@ -15,9 +19,10 @@ const {
}
} = require('./errors')

function ContentTypeParser (bodyLimit) {
function ContentTypeParser (bodyLimit, onProtoPoisoning) {
this[kDefaultJsonParse] = getDefaultJsonParser(onProtoPoisoning)
this.customParsers = {}
this.customParsers['application/json'] = new Parser(true, false, bodyLimit, defaultJsonParser)
this.customParsers['application/json'] = new Parser(true, false, bodyLimit, this[kDefaultJsonParse])
this.customParsers['text/plain'] = new Parser(true, false, bodyLimit, defaultPlainTextParser)
this.parserList = ['application/json', 'text/plain']
this.cache = lru(100)
Expand Down Expand Up @@ -58,7 +63,7 @@ ContentTypeParser.prototype.add = function (contentType, opts, parserFn) {

ContentTypeParser.prototype.hasParser = function (contentType) {
if (contentType === 'application/json') {
return this.customParsers['application/json'].fn !== defaultJsonParser
return this.customParsers['application/json'].fn !== this[kDefaultJsonParse]
}
if (contentType === 'text/plain') {
return this.customParsers['text/plain'].fn !== defaultPlainTextParser
Expand Down Expand Up @@ -181,18 +186,22 @@ function rawBody (request, reply, options, parser, done) {
}
}

function defaultJsonParser (req, body, done) {
if (body === '' || body == null) {
return done(new FST_ERR_CTP_EMPTY_JSON_BODY(), undefined)
}
function getDefaultJsonParser (onProtoPoisoning) {
return defaultJsonParser

function defaultJsonParser (req, body, done) {
if (body === '' || body == null) {
return done(new FST_ERR_CTP_EMPTY_JSON_BODY(), undefined)
}

try {
var json = JSON.parse(body)
} catch (err) {
err.statusCode = 400
return done(err, undefined)
try {
var json = Bourne.parse(body, { protoAction: onProtoPoisoning })
} catch (err) {
err.statusCode = 400
return done(err, undefined)
}
done(null, json)
}
done(null, json)
}

function defaultPlainTextParser (req, body, done) {
Expand Down
1 change: 1 addition & 0 deletions lib/symbols.js
Expand Up @@ -14,6 +14,7 @@ const keys = {
kCanSetNotFoundHandler: Symbol('fastify.canSetNotFoundHandler'),
kFourOhFourLevelInstance: Symbol('fastify.404LogLevelInstance'),
kFourOhFourContext: Symbol('fastify.404ContextKey'),
kDefaultJsonParse: Symbol('fastify.defaultJSONParse'),
kReplySerializer: Symbol('fastify.reply.serializer'),
kReplyIsError: Symbol('fastify.reply.isError'),
kReplyHeaders: Symbol('fastify.reply.headers'),
Expand Down
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -119,6 +119,7 @@
"abstract-logging": "^1.0.0",
"ajv": "^6.6.0",
"avvio": "^6.0.0",
"bourne": "^1.1.0",
"fast-json-stringify": "^1.9.1",
"find-my-way": "^1.15.4",
"flatstr": "^1.0.8",
Expand Down
83 changes: 83 additions & 0 deletions test/proto-poisoning.test.js
@@ -0,0 +1,83 @@
'use strict'

const Fastify = require('..')
const sget = require('simple-get').concat
const t = require('tap')
const test = t.test

test('proto-poisoning error', t => {
t.plan(3)

const fastify = Fastify()
t.tearDown(fastify.close.bind(fastify))

fastify.post('/', (request, reply) => {
t.fail('handler should not be called')
})

fastify.listen(0, function (err) {
t.error(err)

sget({
method: 'POST',
url: 'http://localhost:' + fastify.server.address().port,
headers: { 'Content-Type': 'application/json' },
body: '{ "__proto__": { "a": 42 } }'
}, (err, response, body) => {
t.error(err)
t.strictEqual(response.statusCode, 400)
})
})
})

test('proto-poisoning remove', t => {
t.plan(4)

const fastify = Fastify({ onProtoPoisoning: 'remove' })
t.tearDown(fastify.close.bind(fastify))

fastify.post('/', (request, reply) => {
t.equal(undefined, Object.assign({}, request.body).a)
reply.send({ ok: true })
})

fastify.listen(0, function (err) {
t.error(err)

sget({
method: 'POST',
url: 'http://localhost:' + fastify.server.address().port,
headers: { 'Content-Type': 'application/json' },
body: '{ "__proto__": { "a": 42 }, "b": 42 }'
}, (err, response, body) => {
t.error(err)
t.strictEqual(response.statusCode, 200)
})
})
})

test('proto-poisoning ignore', t => {
t.plan(4)

const fastify = Fastify({ onProtoPoisoning: 'ignore' })
t.tearDown(fastify.close.bind(fastify))

fastify.post('/', (request, reply) => {
t.equal(42, Object.assign({}, request.body).a)
reply.send({ ok: true })
})

fastify.listen(0, function (err) {
t.error(err)

sget({
method: 'POST',
url: 'http://localhost:' + fastify.server.address().port,
headers: { 'Content-Type': 'application/json' },
body: '{ "__proto__": { "a": 42 }, "b": 42 }'
}, (err, response, body) => {
t.error(err)
t.strictEqual(response.statusCode, 200)
})
})
})