Skip to content

Commit 3086902

Browse files
ljharbljharb
committed
[Fix] ensure arrayLength applies to [] notation as well
1 parent fc7930e commit 3086902

File tree

4 files changed

+303
-11
lines changed

4 files changed

+303
-11
lines changed

‎lib/parse.js‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,12 @@ var parseValues = function parseQueryStringValues(str, options) {
133133
if (key !== null) {
134134
var existing = has.call(obj, key);
135135
if (existing && options.duplicates === 'combine') {
136-
obj[key] = utils.combine(obj[key], val);
136+
obj[key] = utils.combine(
137+
obj[key],
138+
val,
139+
options.arrayLimit,
140+
options.plainObjects
141+
);
137142
} else if (!existing || options.duplicates === 'last') {
138143
obj[key] = val;
139144
}
@@ -157,9 +162,19 @@ var parseObject = function (chain, val, options, valuesParsed) {
157162
var root = chain[i];
158163

159164
if (root === '[]' && options.parseArrays) {
160-
obj = options.allowEmptyArrays && (leaf === '' || (options.strictNullHandling && leaf === null))
161-
? []
162-
: utils.combine([], leaf);
165+
if (utils.isOverflow(leaf)) {
166+
// leaf is already an overflow object, preserve it
167+
obj = leaf;
168+
} else {
169+
obj = options.allowEmptyArrays && (leaf === '' || (options.strictNullHandling && leaf === null))
170+
? []
171+
: utils.combine(
172+
[],
173+
leaf,
174+
options.arrayLimit,
175+
options.plainObjects
176+
);
177+
}
163178
} else {
164179
obj = options.plainObjects ? { __proto__: null } : {};
165180
var cleanRoot = root.charAt(0) === '[' && root.charAt(root.length - 1) === ']' ? root.slice(1, -1) : root;

‎lib/utils.js‎

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,32 @@
11
'use strict';
22

33
var formats = require('./formats');
4+
var getSideChannel = require('side-channel');
45

56
var has = Object.prototype.hasOwnProperty;
67
var isArray = Array.isArray;
78

9+
// Track objects created from arrayLimit overflow using side-channel
10+
// Stores the current max numeric index for O(1) lookup
11+
var overflowChannel = getSideChannel();
12+
13+
var markOverflow = function markOverflow(obj, maxIndex) {
14+
overflowChannel.set(obj, maxIndex);
15+
return obj;
16+
};
17+
18+
var isOverflow = function isOverflow(obj) {
19+
return overflowChannel.has(obj);
20+
};
21+
22+
var getMaxIndex = function getMaxIndex(obj) {
23+
return overflowChannel.get(obj);
24+
};
25+
26+
var setMaxIndex = function setMaxIndex(obj, maxIndex) {
27+
overflowChannel.set(obj, maxIndex);
28+
};
29+
830
var hexTable = (function () {
931
var array = [];
1032
for (var i = 0; i < 256; ++i) {
@@ -54,7 +76,12 @@ var merge = function merge(target, source, options) {
5476
if (isArray(target)) {
5577
target.push(source);
5678
} else if (target && typeof target === 'object') {
57-
if (
79+
if (isOverflow(target)) {
80+
// Add at next numeric index for overflow objects
81+
var newIndex = getMaxIndex(target) + 1;
82+
target[newIndex] = source;
83+
setMaxIndex(target, newIndex);
84+
} else if (
5885
(options && (options.plainObjects || options.allowPrototypes))
5986
|| !has.call(Object.prototype, source)
6087
) {
@@ -68,6 +95,18 @@ var merge = function merge(target, source, options) {
6895
}
6996

7097
if (!target || typeof target !== 'object') {
98+
if (isOverflow(source)) {
99+
// Create new object with target at 0, source values shifted by 1
100+
var sourceKeys = Object.keys(source);
101+
var result = options && options.plainObjects
102+
? { __proto__: null, 0: target }
103+
: { 0: target };
104+
for (var m = 0; m < sourceKeys.length; m++) {
105+
var oldKey = parseInt(sourceKeys[m], 10);
106+
result[oldKey + 1] = source[sourceKeys[m]];
107+
}
108+
return markOverflow(result, getMaxIndex(source) + 1);
109+
}
71110
return [target].concat(source);
72111
}
73112

@@ -239,8 +278,20 @@ var isBuffer = function isBuffer(obj) {
239278
return !!(obj.constructor && obj.constructor.isBuffer && obj.constructor.isBuffer(obj));
240279
};
241280

242-
var combine = function combine(a, b) {
243-
return [].concat(a, b);
281+
var combine = function combine(a, b, arrayLimit, plainObjects) {
282+
// If 'a' is already an overflow object, add to it
283+
if (isOverflow(a)) {
284+
var newIndex = getMaxIndex(a) + 1;
285+
a[newIndex] = b;
286+
setMaxIndex(a, newIndex);
287+
return a;
288+
}
289+
290+
var result = [].concat(a, b);
291+
if (result.length > arrayLimit) {
292+
return markOverflow(arrayToObject(result, { plainObjects: plainObjects }), result.length - 1);
293+
}
294+
return result;
244295
};
245296

246297
var maybeMap = function maybeMap(val, fn) {
@@ -262,6 +313,7 @@ module.exports = {
262313
decode: decode,
263314
encode: encode,
264315
isBuffer: isBuffer,
316+
isOverflow: isOverflow,
265317
isRegExp: isRegExp,
266318
maybeMap: maybeMap,
267319
merge: merge

‎test/parse.js‎

Lines changed: 110 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -235,11 +235,11 @@ test('parse()', function (t) {
235235
st.deepEqual(qs.parse('a=b&a[0]=c'), { a: ['b', 'c'] });
236236

237237
st.deepEqual(qs.parse('a[1]=b&a=c', { arrayLimit: 20 }), { a: ['b', 'c'] });
238-
st.deepEqual(qs.parse('a[]=b&a=c', { arrayLimit: 0 }), { a: ['b', 'c'] });
238+
st.deepEqual(qs.parse('a[]=b&a=c', { arrayLimit: 0 }), { a: { 0: 'b', 1: 'c' } });
239239
st.deepEqual(qs.parse('a[]=b&a=c'), { a: ['b', 'c'] });
240240

241241
st.deepEqual(qs.parse('a=b&a[1]=c', { arrayLimit: 20 }), { a: ['b', 'c'] });
242-
st.deepEqual(qs.parse('a=b&a[]=c', { arrayLimit: 0 }), { a: ['b', 'c'] });
242+
st.deepEqual(qs.parse('a=b&a[]=c', { arrayLimit: 0 }), { a: { 0: 'b', 1: 'c' } });
243243
st.deepEqual(qs.parse('a=b&a[]=c'), { a: ['b', 'c'] });
244244

245245
st.end();
@@ -364,7 +364,7 @@ test('parse()', function (t) {
364364
);
365365
st.deepEqual(
366366
qs.parse('a[]=b&a[]&a[]=c&a[]=', { strictNullHandling: true, arrayLimit: 0 }),
367-
{ a: ['b', null, 'c', ''] },
367+
{ a: { 0: 'b', 1: null, 2: 'c', 3: '' } },
368368
'with arrayLimit 0 + array brackets: null then empty string works'
369369
);
370370

@@ -375,7 +375,7 @@ test('parse()', function (t) {
375375
);
376376
st.deepEqual(
377377
qs.parse('a[]=b&a[]=&a[]=c&a[]', { strictNullHandling: true, arrayLimit: 0 }),
378-
{ a: ['b', '', 'c', null] },
378+
{ a: { 0: 'b', 1: '', 2: 'c', 3: null } },
379379
'with arrayLimit 0 + array brackets: empty string then null works'
380380
);
381381

@@ -1288,3 +1288,109 @@ test('qs strictDepth option - non-throw cases', function (t) {
12881288
st.end();
12891289
});
12901290
});
1291+
1292+
test('DOS', function (t) {
1293+
var arr = [];
1294+
for (var i = 0; i < 105; i++) {
1295+
arr[arr.length] = 'x';
1296+
}
1297+
var attack = 'a[]=' + arr.join('&a[]=');
1298+
var result = qs.parse(attack, { arrayLimit: 100 });
1299+
1300+
t.notOk(Array.isArray(result.a), 'arrayLimit is respected: result is an object, not an array');
1301+
t.equal(Object.keys(result.a).length, 105, 'all values are preserved');
1302+
1303+
t.end();
1304+
});
1305+
1306+
test('arrayLimit boundary conditions', function (t) {
1307+
t.test('exactly at the limit stays as array', function (st) {
1308+
var result = qs.parse('a[]=1&a[]=2&a[]=3', { arrayLimit: 3 });
1309+
st.ok(Array.isArray(result.a), 'result is an array when exactly at limit');
1310+
st.deepEqual(result.a, ['1', '2', '3'], 'all values present');
1311+
st.end();
1312+
});
1313+
1314+
t.test('one over the limit converts to object', function (st) {
1315+
var result = qs.parse('a[]=1&a[]=2&a[]=3&a[]=4', { arrayLimit: 3 });
1316+
st.notOk(Array.isArray(result.a), 'result is not an array when over limit');
1317+
st.deepEqual(result.a, { 0: '1', 1: '2', 2: '3', 3: '4' }, 'all values preserved as object');
1318+
st.end();
1319+
});
1320+
1321+
t.test('arrayLimit 1 with two values', function (st) {
1322+
var result = qs.parse('a[]=1&a[]=2', { arrayLimit: 1 });
1323+
st.notOk(Array.isArray(result.a), 'result is not an array');
1324+
st.deepEqual(result.a, { 0: '1', 1: '2' }, 'both values preserved');
1325+
st.end();
1326+
});
1327+
1328+
t.end();
1329+
});
1330+
1331+
test('mixed array and object notation', function (t) {
1332+
t.test('array brackets with object key - under limit', function (st) {
1333+
st.deepEqual(
1334+
qs.parse('a[]=b&a[c]=d'),
1335+
{ a: { 0: 'b', c: 'd' } },
1336+
'mixing [] and [key] converts to object'
1337+
);
1338+
st.end();
1339+
});
1340+
1341+
t.test('array index with object key - under limit', function (st) {
1342+
st.deepEqual(
1343+
qs.parse('a[0]=b&a[c]=d'),
1344+
{ a: { 0: 'b', c: 'd' } },
1345+
'mixing [0] and [key] produces object'
1346+
);
1347+
st.end();
1348+
});
1349+
1350+
t.test('plain value with array brackets - under limit', function (st) {
1351+
st.deepEqual(
1352+
qs.parse('a=b&a[]=c', { arrayLimit: 20 }),
1353+
{ a: ['b', 'c'] },
1354+
'plain value combined with [] stays as array under limit'
1355+
);
1356+
st.end();
1357+
});
1358+
1359+
t.test('array brackets with plain value - under limit', function (st) {
1360+
st.deepEqual(
1361+
qs.parse('a[]=b&a=c', { arrayLimit: 20 }),
1362+
{ a: ['b', 'c'] },
1363+
'[] combined with plain value stays as array under limit'
1364+
);
1365+
st.end();
1366+
});
1367+
1368+
t.test('plain value with array index - under limit', function (st) {
1369+
st.deepEqual(
1370+
qs.parse('a=b&a[0]=c', { arrayLimit: 20 }),
1371+
{ a: ['b', 'c'] },
1372+
'plain value combined with [0] stays as array under limit'
1373+
);
1374+
st.end();
1375+
});
1376+
1377+
t.test('multiple plain values with duplicates combine', function (st) {
1378+
st.deepEqual(
1379+
qs.parse('a=b&a=c&a=d', { arrayLimit: 20 }),
1380+
{ a: ['b', 'c', 'd'] },
1381+
'duplicate plain keys combine into array'
1382+
);
1383+
st.end();
1384+
});
1385+
1386+
t.test('multiple plain values exceeding limit', function (st) {
1387+
st.deepEqual(
1388+
qs.parse('a=b&a=c&a=d', { arrayLimit: 2 }),
1389+
{ a: { 0: 'b', 1: 'c', 2: 'd' } },
1390+
'duplicate plain keys convert to object when exceeding limit'
1391+
);
1392+
st.end();
1393+
});
1394+
1395+
t.end();
1396+
});

0 commit comments

Comments
 (0)