Skip to content

Commit 1f78ce4

Browse files
authored
fix: prevent DecodeOptions.listLimit bypass in bracket notation to mitigate potential DoS via memory exhaustion (#46)
1 parent be652e1 commit 1f78ce4

File tree

8 files changed

+385
-48
lines changed

8 files changed

+385
-48
lines changed

lib/src/extensions/decode.dart

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ extension _$Decode on QS {
192192
// Duplicate key policy: combine/first/last (default: combine).
193193
final bool existing = obj.containsKey(key);
194194
if (existing && options.duplicates == Duplicates.combine) {
195-
obj[key] = Utils.combine(obj[key], val);
195+
obj[key] = Utils.combine(obj[key], val, listLimit: options.listLimit);
196196
} else if (!existing || options.duplicates == Duplicates.last) {
197197
obj[key] = val;
198198
}
@@ -209,7 +209,8 @@ extension _$Decode on QS {
209209
/// - When `parseLists` is false, numeric segments are treated as string keys.
210210
/// - When `allowEmptyLists` is true, an empty string (or `null` under
211211
/// `strictNullHandling`) under a `[]` segment yields an empty list.
212-
/// - `listLimit` applies to explicit numeric indices as an upper bound.
212+
/// - `listLimit` applies to explicit numeric indices and list growth via `[]`;
213+
/// when exceeded, lists are converted into maps with string indices.
213214
/// - A negative `listLimit` disables numeric‑index parsing (bracketed numbers become map keys).
214215
/// Empty‑bracket pushes (`[]`) still create lists here; this method does not enforce
215216
/// `throwOnLimitExceeded` for that path. Comma‑split growth (if any) has already been
@@ -263,10 +264,16 @@ extension _$Decode on QS {
263264
// Anonymous list segment `[]` — either an empty list (when allowed) or a
264265
// single-element list with the leaf combined in.
265266
if (root == '[]' && options.parseLists) {
266-
obj = options.allowEmptyLists &&
267-
(leaf == '' || (options.strictNullHandling && leaf == null))
268-
? List<dynamic>.empty(growable: true)
269-
: Utils.combine([], leaf);
267+
if (Utils.isOverflow(leaf)) {
268+
// leaf can already be overflow (e.g. duplicates combine/listLimit),
269+
// so preserve it instead of re-wrapping into a list.
270+
obj = leaf;
271+
} else {
272+
obj = options.allowEmptyLists &&
273+
(leaf == '' || (options.strictNullHandling && leaf == null))
274+
? List<dynamic>.empty(growable: true)
275+
: Utils.combine([], leaf, listLimit: options.listLimit);
276+
}
270277
} else {
271278
obj = <String, dynamic>{};
272279
// Normalize bracketed segments ("[k]"). Note: depending on how key decoding is configured,

lib/src/models/decode_options.dart

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,13 @@ final class DecodeOptions with EquatableMixin {
9898
/// `a[]=` without coercing or discarding them.
9999
final bool allowEmptyLists;
100100

101-
/// Maximum list index that will be honored when decoding bracket indices.
101+
/// Maximum list size/index that will be honored when decoding bracket lists.
102102
///
103103
/// Keys like `a[9999999]` can cause excessively large sparse lists; above
104-
/// this limit, indices are treated as string map keys instead.
104+
/// this limit, indices are treated as string map keys instead. The same
105+
/// limit also applies to empty-bracket pushes (`a[]`) and duplicate combines:
106+
/// once growth exceeds the limit, the list is converted to a map with string
107+
/// indices to preserve values (matching Node `qs` arrayLimit semantics).
105108
///
106109
/// **Negative values:** passing a negative `listLimit` (e.g. `-1`) disables
107110
/// numeric‑index parsing entirely — any bracketed number like `a[0]` or

lib/src/qs.dart

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,6 @@ final class QS {
3737
/// * a query [String] (e.g. `"a=1&b[c]=2"`), or
3838
/// * a pre-tokenized `Map<String, dynamic>` produced by a custom tokenizer.
3939
/// - When `input` is `null` or the empty string, `{}` is returned.
40-
/// - If [DecodeOptions.parseLists] is `true` and the number of top‑level
41-
/// parameters exceeds [DecodeOptions.listLimit], list parsing is
42-
/// temporarily disabled for this call to bound memory (mirrors Node `qs`).
4340
/// - Throws [ArgumentError] if `input` is neither a `String` nor a
4441
/// `Map<String, dynamic>`.
4542
///
@@ -67,16 +64,6 @@ final class QS {
6764
? _$Decode._parseQueryStringValues(input, options)
6865
: input;
6966

70-
// Guardrail: if the top-level parameter count is large, temporarily disable
71-
// list parsing to keep memory bounded (matches Node `qs`). Only apply for
72-
// raw string inputs, not for pre-tokenized maps.
73-
if (input is String &&
74-
options.parseLists &&
75-
options.listLimit > 0 &&
76-
(tempObj?.length ?? 0) > options.listLimit) {
77-
options = options.copyWith(parseLists: false);
78-
}
79-
8067
Map<String, dynamic> obj = {};
8168

8269
// Merge each parsed key into the accumulator using the same rules as Node `qs`.

lib/src/utils.dart

Lines changed: 122 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,42 @@ part 'constants/hex_table.dart';
2828
final class Utils {
2929
static const int _segmentLimit = 1024;
3030

31+
/// Tracks array-overflow objects (from listLimit) without mutating user data.
32+
static final Expando<int> _overflowIndex = Expando<int>('qsOverflowIndex');
33+
34+
/// Marks a map as an overflow container with the given max index.
35+
@internal
36+
@visibleForTesting
37+
static Map<String, dynamic> markOverflow(
38+
Map<String, dynamic> obj,
39+
int maxIndex,
40+
) {
41+
_overflowIndex[obj] = maxIndex;
42+
return obj;
43+
}
44+
45+
/// Returns `true` if the given object is marked as an overflow container.
46+
@internal
47+
static bool isOverflow(dynamic obj) =>
48+
obj is Map && _overflowIndex[obj] != null;
49+
50+
/// Returns the tracked max numeric index for an overflow map, or -1 if absent.
51+
static int _getOverflowIndex(Map obj) => _overflowIndex[obj] ?? -1;
52+
53+
/// Updates the tracked max numeric index for an overflow map.
54+
static void _setOverflowIndex(Map obj, int maxIndex) {
55+
_overflowIndex[obj] = maxIndex;
56+
}
57+
58+
/// Returns the larger of the current max and the parsed numeric key (if any).
59+
static int _updateOverflowMax(int current, String key) {
60+
final int? parsed = int.tryParse(key);
61+
if (parsed == null || parsed < 0) {
62+
return current;
63+
}
64+
return parsed > current ? parsed : current;
65+
}
66+
3167
/// Deeply merges `source` into `target` while preserving insertion order
3268
/// and list semantics used by `qs`.
3369
///
@@ -127,12 +163,17 @@ final class Utils {
127163
}
128164
} else if (target is Map) {
129165
if (source is Iterable) {
130-
target = <String, dynamic>{
131-
for (final MapEntry entry in target.entries)
132-
entry.key.toString(): entry.value,
166+
final Map<String, dynamic> sourceMap = {
133167
for (final (int i, dynamic item) in source.indexed)
134168
if (item is! Undefined) i.toString(): item
135169
};
170+
return merge(target, sourceMap, options);
171+
}
172+
if (isOverflow(target)) {
173+
final int newIndex = _getOverflowIndex(target) + 1;
174+
target[newIndex.toString()] = source;
175+
_setOverflowIndex(target, newIndex);
176+
return target;
136177
}
137178
} else if (source != null) {
138179
if (target is! Iterable && source is Iterable) {
@@ -146,11 +187,40 @@ final class Utils {
146187

147188
if (target == null || target is! Map) {
148189
if (target is Iterable) {
149-
return Map<String, dynamic>.of({
190+
final Map<String, dynamic> mergeTarget = {
150191
for (final (int i, dynamic item) in target.indexed)
151192
if (item is! Undefined) i.toString(): item,
152-
...source,
153-
});
193+
};
194+
for (final MapEntry entry in source.entries) {
195+
final String key = entry.key.toString();
196+
if (mergeTarget.containsKey(key)) {
197+
mergeTarget[key] = merge(
198+
mergeTarget[key],
199+
entry.value,
200+
options,
201+
);
202+
} else {
203+
mergeTarget[key] = entry.value;
204+
}
205+
}
206+
return mergeTarget;
207+
}
208+
209+
if (isOverflow(source)) {
210+
final int sourceMax = _getOverflowIndex(source);
211+
final Map<String, dynamic> result = {
212+
if (target != null) '0': target,
213+
};
214+
for (final MapEntry entry in source.entries) {
215+
final String key = entry.key.toString();
216+
final int? oldIndex = int.tryParse(key);
217+
if (oldIndex == null) {
218+
result[key] = entry.value;
219+
} else {
220+
result[(oldIndex + 1).toString()] = entry.value;
221+
}
222+
}
223+
return markOverflow(result, sourceMax + 1);
154224
}
155225

156226
return [
@@ -165,6 +235,9 @@ final class Utils {
165235
];
166236
}
167237

238+
final bool targetOverflow = isOverflow(target);
239+
int? overflowMax = targetOverflow ? _getOverflowIndex(target) : null;
240+
168241
Map<String, dynamic> mergeTarget = target is Iterable && source is! Iterable
169242
? {
170243
for (final (int i, dynamic item) in (target as Iterable).indexed)
@@ -176,6 +249,9 @@ final class Utils {
176249
};
177250

178251
for (final MapEntry entry in source.entries) {
252+
if (overflowMax != null) {
253+
overflowMax = _updateOverflowMax(overflowMax, entry.key.toString());
254+
}
179255
mergeTarget.update(
180256
entry.key.toString(),
181257
(value) => merge(
@@ -186,6 +262,9 @@ final class Utils {
186262
ifAbsent: () => entry.value,
187263
);
188264
}
265+
if (overflowMax != null) {
266+
markOverflow(mergeTarget, overflowMax);
267+
}
189268
return mergeTarget;
190269
}
191270

@@ -577,17 +656,47 @@ final class Utils {
577656
return root;
578657
}
579658

580-
/// Concatenates two values as a typed `List<T>`, spreading iterables.
659+
/// Concatenates two values, spreading iterables.
660+
///
661+
/// When [listLimit] is provided and exceeded, returns a map with string keys.
662+
/// Any throwing behavior is enforced earlier during parsing, matching Node `qs`.
663+
///
664+
/// **Note:** If [a] is already an overflow object, this method mutates [a]
665+
/// in place by appending entries from [b].
581666
///
582667
/// Examples:
583668
/// ```dart
584-
/// combine&lt;int&gt;([1,2], 3); // [1,2,3]
585-
/// combine&lt;String&gt;('a', ['b','c']); // ['a','b','c']
669+
/// combine([1,2], 3); // [1,2,3]
670+
/// combine('a', ['b','c']); // ['a','b','c']
586671
/// ```
587-
static List<T> combine<T>(dynamic a, dynamic b) => <T>[
588-
if (a is Iterable<T>) ...a else a,
589-
if (b is Iterable<T>) ...b else b,
590-
];
672+
static dynamic combine(dynamic a, dynamic b, {int? listLimit}) {
673+
if (isOverflow(a)) {
674+
int newIndex = _getOverflowIndex(a);
675+
if (b is Iterable) {
676+
for (final item in b) {
677+
newIndex++;
678+
a[newIndex.toString()] = item;
679+
}
680+
} else {
681+
newIndex++;
682+
a[newIndex.toString()] = b;
683+
}
684+
_setOverflowIndex(a, newIndex);
685+
return a;
686+
}
687+
688+
final List<dynamic> result = <dynamic>[
689+
if (a is Iterable) ...a else a,
690+
if (b is Iterable) ...b else b,
691+
];
692+
693+
if (listLimit != null && listLimit >= 0 && result.length > listLimit) {
694+
final Map<String, dynamic> overflow = createIndexMap(result);
695+
return markOverflow(overflow, result.length - 1);
696+
}
697+
698+
return result;
699+
}
591700

592701
/// Applies `fn` to a scalar or maps it over an iterable, returning the result.
593702
///

test/comparison/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@
55
"author": "Klemen Tusar",
66
"license": "BSD-3-Clause",
77
"dependencies": {
8-
"qs": "^6.12.1"
8+
"qs": "^6.14.1"
99
}
1010
}

0 commit comments

Comments
 (0)