Skip to content

Commit 0c180a4

Browse files
committed
[Fix] stringify: skip null/undefined filter-array entries instead of crashing in encoder
When `filter` is an array, its elements become `objKeys` directly and the top-level loop passes each entry as the recursive `prefix` argument without coercion. A `null` or `undefined` entry reached `utils.encode` as the `str` argument and `str.length` threw `TypeError`. The crash requires two conditions to align: the filter array must contain `null`/`undefined`, and the object being stringified must have a literal `'null'`/`'undefined'` property (otherwise `obj[key]` is `undefined` and the typeof-undefined early-return at stringify.js:137 fires before the encoder is called) Both are reachable from an attacker controlled input in applications that stringify user-submitted JSON with user-selected fields. Fix this up by skipping `null`/`undefined` filter entries at the top-level loop. This somewhat matches `JSON.stringify` replacer-array semantics, which silently ignore non-string/non-number entries. `Object.keys` never yields `null`/`undefined`, so the guard only affects user-supplied filter arrays. The inner recursive loop already coerces `key` via `String()` when building `keyPrefix`, so it never passes a raw `null` to the encoder.
1 parent 3a8b94a commit 0c180a4

2 files changed

Lines changed: 34 additions & 0 deletions

File tree

lib/stringify.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,11 @@ module.exports = function (object, opts) {
312312
var sideChannel = getSideChannel();
313313
for (var i = 0; i < objKeys.length; ++i) {
314314
var key = objKeys[i];
315+
316+
if (typeof key === 'undefined' || key === null) {
317+
continue;
318+
}
319+
315320
var value = obj[key];
316321

317322
if (options.skipNulls && value === null) {

test/stringify.js

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -825,6 +825,35 @@ test('stringify()', function (t) {
825825
st.end();
826826
});
827827

828+
t.test('skips null/undefined entries in filter=array', function (st) {
829+
st.doesNotThrow(
830+
function () { qs.stringify({ a: 'b', undefined: 'x' }, { filter: ['a', undefined] }); },
831+
'does not pass a raw undefined filter entry to the encoder'
832+
);
833+
st.doesNotThrow(
834+
function () { qs.stringify({ a: 'b', 'null': 'x' }, { filter: ['a', null] }); },
835+
'does not pass a raw null filter entry to the encoder'
836+
);
837+
838+
st.equal(
839+
qs.stringify({ a: 'b', undefined: 'x', c: 'd' }, { filter: ['a', undefined, 'c'] }),
840+
'a=b&c=d',
841+
'undefined filter entry is skipped, remaining keys are kept'
842+
);
843+
st.equal(
844+
qs.stringify({ a: 'b', 'null': 'x', c: 'd' }, { filter: ['a', null, 'c'] }),
845+
'a=b&c=d',
846+
'null filter entry is skipped, remaining keys are kept'
847+
);
848+
st.equal(
849+
qs.stringify({ a: 'b', 'null': 'x' }, { filter: [null] }),
850+
'',
851+
'filter array containing only null yields empty string'
852+
);
853+
854+
st.end();
855+
});
856+
828857
t.test('supports custom representations when filter=function', function (st) {
829858
var calls = 0;
830859
var obj = { a: 'b', c: 'd', e: { f: new Date(1257894000000) } };

0 commit comments

Comments
 (0)