diff --git a/src/shielding/middleware/handle-invalid-query-string-values.js b/src/shielding/middleware/handle-invalid-query-string-values.js index 142069d4cbd0..fadc45ee90e2 100644 --- a/src/shielding/middleware/handle-invalid-query-string-values.js +++ b/src/shielding/middleware/handle-invalid-query-string-values.js @@ -32,7 +32,7 @@ const RECOGNIZED_VALUES_KEYS = new Set(Object.keys(RECOGNIZED_VALUES)) export default function handleInvalidQuerystringValues(req, res, next) { const { method, query } = req if (method === 'GET' || method === 'HEAD') { - for (const key of Object.keys(query)) { + for (const [key, value] of Object.entries(query)) { if (RECOGNIZED_VALUES_KEYS.has(key)) { const validValues = RECOGNIZED_VALUES[key] const values = Array.isArray(query[key]) ? query[key] : [query[key]] @@ -66,6 +66,17 @@ export default function handleInvalidQuerystringValues(req, res, next) { return } } + + // For example ?foo[bar]=baz (but not ?foo=bar&foo=baz) + if (value instanceof Object && !Array.isArray(value)) { + const message = `Invalid query string key (${key})` + defaultCacheControl(res) + res.status(400).send(message) + + const tags = ['response:400', `path:${req.path}`, `key:${key}`] + statsd.increment(STATSD_KEY, 1, tags) + return + } } } diff --git a/src/shielding/tests/invalid-querystrings.js b/src/shielding/tests/invalid-querystrings.js index cdf9b95667ac..59fb7dac2cc1 100644 --- a/src/shielding/tests/invalid-querystrings.js +++ b/src/shielding/tests/invalid-querystrings.js @@ -57,11 +57,18 @@ describe('invalid query strings', () => { } }) + test('query string keys with single square brackets', async () => { + const url = `/en?query[foo]=bar` + const res = await get(url) + expect(res.statusCode).toBe(400) + expect(res.body).toMatch('Invalid query string key (query)') + }) + test('query string keys with square brackets', async () => { const url = `/?constructor[foo][bar]=buz` const res = await get(url) - expect(res.statusCode).toBe(302) - expect(res.headers.location).toBe('/en') + expect(res.statusCode).toBe(400) + expect(res.body).toMatch('Invalid query string key (constructor)') }) test('bad tool query string with Chinese URL-encoded characters', async () => {