Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 16 additions & 9 deletions lib/utils/queryparams.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,24 @@ module.exports = (req, url) => {
return
}

// Process query parameters with optimized URLSearchParams handling
const searchParams = new URLSearchParams(search.replace(/\[\]=/g, '='))
// Only rewrite array notation (a[]=1 -> a=1) when it is actually present,
// avoiding a regex scan/allocation on the common query-string case.
const searchParams = new URLSearchParams(
search.indexOf('[]=') === -1 ? search : search.replace(/\[\]=/g, '=')
)

for (const [name, value] of searchParams.entries()) {
// Split parameter name into segments by dot or bracket notation
/* eslint-disable-next-line */
const segments = name.split(/[\.\[\]]+/).filter(Boolean)

// Check each segment against the dangerous properties set
if (segments.some(segment => DANGEROUS_PROPERTIES.has(segment))) {
continue // Skip dangerous property names
// Prototype-pollution guard. The segment split/filter allocates per
// parameter, so it only runs for the rare names that could actually carry a
// dangerous segment. Every dangerous key ('__proto__', 'prototype',
// 'constructor') contains 'proto' or 'constructor' as a substring.
if (name.indexOf('proto') !== -1 || name.indexOf('constructor') !== -1) {
// Split parameter name into segments by dot or bracket notation
/* eslint-disable-next-line */
const segments = name.split(/[\.\[\]]+/).filter(Boolean)
if (segments.some(segment => DANGEROUS_PROPERTIES.has(segment))) {
continue // Skip dangerous property names
}
}

const existing = query[name]
Expand Down
17 changes: 17 additions & 0 deletions tests/router-coverage.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,23 @@ describe('0http - Router Coverage', () => {
expect(req.path).to.equal('/path')
expect(req.query).to.deep.equal({ param: '' })
})

it('should group repeated and array-notation parameters', () => {
const req = {}
queryparams(req, '/path?id[]=1&id[]=2&name=a&name=b&tag=x')
expect(req.query).to.deep.equal({ id: ['1', '2'], name: ['a', 'b'], tag: 'x' })
})

it('should strip dangerous prototype-pollution keys', () => {
const req = {}
queryparams(req, '/path?__proto__=evil&prototype=z&user.constructor=bad&a[__proto__]=x&ok=1')
// Dangerous keys are dropped; the query object has no prototype.
expect(Object.getPrototypeOf(req.query)).to.equal(null)
expect(req.query.ok).to.equal('1')
expect(Object.keys(req.query)).to.deep.equal(['ok'])
// No pollution leaked onto Object.prototype.
expect(({}).evil).to.equal(undefined)
})
})

describe('Next Middleware Executor', () => {
Expand Down
Loading