Skip to content

Commit

Permalink
Merge pull request from GHSA-455w-c45v-86rg
Browse files Browse the repository at this point in the history
* Add safeguard against malicious content types

* Update lib/contentTypeParser.js

Co-authored-by: Manuel Spigolon <behemoth89@gmail.com>

* Apply spelling fixes from code review

Co-authored-by: James Sumners <james@sumners.email>

Co-authored-by: Manuel Spigolon <behemoth89@gmail.com>
Co-authored-by: Frazer Smith <frazer.dev@outlook.com>
Co-authored-by: James Sumners <james@sumners.email>
  • Loading branch information
4 people committed Oct 10, 2022
1 parent 5053ad9 commit fbb07e8
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 21 deletions.
39 changes: 20 additions & 19 deletions lib/contentTypeParser.js
Expand Up @@ -29,9 +29,10 @@ const {

function ContentTypeParser (bodyLimit, onProtoPoisoning, onConstructorPoisoning) {
this[kDefaultJsonParse] = getDefaultJsonParser(onProtoPoisoning, onConstructorPoisoning)
this.customParsers = {}
this.customParsers['application/json'] = new Parser(true, false, bodyLimit, this[kDefaultJsonParse])
this.customParsers['text/plain'] = new Parser(true, false, bodyLimit, defaultPlainTextParser)
// using a map instead of a plain object to avoid prototype hijack attacks
this.customParsers = new Map()
this.customParsers.set('application/json', new Parser(true, false, bodyLimit, this[kDefaultJsonParse]))
this.customParsers.set('text/plain', new Parser(true, false, bodyLimit, defaultPlainTextParser))
this.parserList = ['application/json', 'text/plain']
this.parserRegExpList = []
this.cache = lru(100)
Expand Down Expand Up @@ -62,35 +63,35 @@ ContentTypeParser.prototype.add = function (contentType, opts, parserFn) {
)

if (contentTypeIsString && contentType === '*') {
this.customParsers[''] = parser
this.customParsers.set('', parser)
} else {
if (contentTypeIsString) {
this.parserList.unshift(contentType)
} else {
this.parserRegExpList.unshift(contentType)
}
this.customParsers[contentType] = parser
this.customParsers.set(contentType.toString(), parser)
}
}

ContentTypeParser.prototype.hasParser = function (contentType) {
return contentType in this.customParsers
return this.customParsers.has(typeof contentType === 'string' ? contentType : contentType.toString())
}

ContentTypeParser.prototype.existingParser = function (contentType) {
if (contentType === 'application/json') {
return this.customParsers['application/json'] && this.customParsers['application/json'].fn !== this[kDefaultJsonParse]
if (contentType === 'application/json' && this.customParsers.has(contentType)) {
return this.customParsers.get(contentType).fn !== this[kDefaultJsonParse]
}
if (contentType === 'text/plain') {
return this.customParsers['text/plain'] && this.customParsers['text/plain'].fn !== defaultPlainTextParser
if (contentType === 'text/plain' && this.customParsers.has(contentType)) {
return this.customParsers.get(contentType).fn !== defaultPlainTextParser
}

return contentType in this.customParsers
return this.hasParser(contentType)
}

ContentTypeParser.prototype.getParser = function (contentType) {
if (contentType in this.customParsers) {
return this.customParsers[contentType]
if (this.hasParser(contentType)) {
return this.customParsers.get(contentType)
}

if (this.cache.has(contentType)) {
Expand All @@ -101,7 +102,7 @@ ContentTypeParser.prototype.getParser = function (contentType) {
for (var i = 0; i !== this.parserList.length; ++i) {
const parserName = this.parserList[i]
if (contentType.indexOf(parserName) !== -1) {
const parser = this.customParsers[parserName]
const parser = this.customParsers.get(parserName)
this.cache.set(contentType, parser)
return parser
}
Expand All @@ -111,17 +112,17 @@ ContentTypeParser.prototype.getParser = function (contentType) {
for (var j = 0; j !== this.parserRegExpList.length; ++j) {
const parserRegExp = this.parserRegExpList[j]
if (parserRegExp.test(contentType)) {
const parser = this.customParsers[parserRegExp]
const parser = this.customParsers.get(parserRegExp.toString())
this.cache.set(contentType, parser)
return parser
}
}

return this.customParsers['']
return this.customParsers.get('')
}

ContentTypeParser.prototype.removeAll = function () {
this.customParsers = {}
this.customParsers = new Map()
this.parserRegExpList = []
this.parserList = []
this.cache = lru(100)
Expand All @@ -130,7 +131,7 @@ ContentTypeParser.prototype.removeAll = function () {
ContentTypeParser.prototype.remove = function (contentType) {
if (!(typeof contentType === 'string' || contentType instanceof RegExp)) throw new FST_ERR_CTP_INVALID_TYPE()

delete this.customParsers[contentType]
this.customParsers.delete(contentType.toString())

const parsers = typeof contentType === 'string' ? this.parserList : this.parserRegExpList

Expand Down Expand Up @@ -289,7 +290,7 @@ function Parser (asString, asBuffer, bodyLimit, fn) {
function buildContentTypeParser (c) {
const contentTypeParser = new ContentTypeParser()
contentTypeParser[kDefaultJsonParse] = c[kDefaultJsonParse]
Object.assign(contentTypeParser.customParsers, c.customParsers)
contentTypeParser.customParsers = new Map(c.customParsers.entries())
contentTypeParser.parserList = c.parserList.slice()
return contentTypeParser
}
Expand Down
70 changes: 68 additions & 2 deletions test/content-parser.test.js
Expand Up @@ -196,7 +196,7 @@ test('add', t => {
const contentTypeParser = fastify[keys.kContentTypeParser]

contentTypeParser.add('*', {}, first)
t.equal(contentTypeParser.customParsers[''].fn, first)
t.equal(contentTypeParser.customParsers.get('').fn, first)
})

t.end()
Expand Down Expand Up @@ -306,7 +306,7 @@ test('remove', t => {

contentTypeParser.remove('image/png')

t.same(Object.keys(contentTypeParser.customParsers).length, 2)
t.same(contentTypeParser.customParsers.size, 2)
})

t.end()
Expand All @@ -329,3 +329,69 @@ test('remove all should remove all existing parsers and reset cache', t => {
t.same(contentTypeParser.parserRegExpList.length, 0)
t.same(Object.keys(contentTypeParser.customParsers).length, 0)
})

test('Safeguard against malicious content-type / 1', async t => {
const badNames = Object.getOwnPropertyNames({}.__proto__) // eslint-disable-line
t.plan(badNames.length)

const fastify = Fastify()

fastify.post('/', async () => {
return 'ok'
})

for (const prop of badNames) {
const response = await fastify.inject({
method: 'POST',
path: '/',
headers: {
'content-type': prop
},
body: ''
})

t.same(response.statusCode, 415)
}
})

test('Safeguard against malicious content-type / 2', async t => {
t.plan(1)

const fastify = Fastify()

fastify.post('/', async () => {
return 'ok'
})

const response = await fastify.inject({
method: 'POST',
path: '/',
headers: {
'content-type': '\\u0063\\u006fnstructor'
},
body: ''
})

t.same(response.statusCode, 415)
})

test('Safeguard against malicious content-type / 3', async t => {
t.plan(1)

const fastify = Fastify()

fastify.post('/', async () => {
return 'ok'
})

const response = await fastify.inject({
method: 'POST',
path: '/',
headers: {
'content-type': 'constructor; charset=utf-8'
},
body: ''
})

t.same(response.statusCode, 415)
})

0 comments on commit fbb07e8

Please sign in to comment.