Skip to content

Commit 8c2de54

Browse files
authored
🐛 implement DecodeOptions.list_limit handling in Utils.combine function to prevent DoS via memory exhaustion (#37)
1 parent a5cb011 commit 8c2de54

6 files changed

Lines changed: 448 additions & 20 deletions

File tree

src/qs_codec/decode.py

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
from .enums.duplicates import Duplicates
2626
from .enums.sentinel import Sentinel
2727
from .models.decode_options import DecodeOptions
28+
from .models.overflow_dict import OverflowDict
2829
from .models.undefined import UNDEFINED
2930
from .utils.decode_utils import DecodeUtils
3031
from .utils.utils import Utils
@@ -288,7 +289,7 @@ def _parse_query_string_values(value: str, options: DecodeOptions) -> t.Dict[str
288289

289290
# Combine/overwrite according to the configured duplicates policy.
290291
if existing and options.duplicates == Duplicates.COMBINE:
291-
obj[key] = Utils.combine(obj[key], val)
292+
obj[key] = Utils.combine(obj[key], val, options)
292293
elif not existing or options.duplicates == Duplicates.LAST:
293294
obj[key] = val
294295

@@ -361,10 +362,14 @@ def _parse_object(
361362
root: str = chain[i]
362363

363364
if root == "[]" and options.parse_lists:
364-
if options.allow_empty_lists and (leaf == "" or (options.strict_null_handling and leaf is None)):
365+
if Utils.is_overflow(leaf):
366+
obj = leaf
367+
elif options.allow_empty_lists and (leaf == "" or (options.strict_null_handling and leaf is None)):
365368
obj = []
366369
else:
367370
obj = list(leaf) if isinstance(leaf, (list, tuple)) else [leaf]
371+
if options.list_limit is not None and len(obj) > options.list_limit:
372+
obj = OverflowDict({str(i): x for i, x in enumerate(obj)})
368373
else:
369374
obj = dict()
370375

@@ -389,7 +394,10 @@ def _parse_object(
389394
index = None
390395

391396
if not options.parse_lists and decoded_root == "":
392-
obj = {"0": leaf}
397+
if Utils.is_overflow(leaf):
398+
obj = leaf
399+
else:
400+
obj = {"0": leaf}
393401
elif (
394402
index is not None
395403
and index >= 0
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
"""Overflow marker for list limit conversions."""
2+
3+
from __future__ import annotations
4+
5+
import copy
6+
7+
8+
class OverflowDict(dict):
9+
"""A mutable marker for list overflows when `list_limit` is exceeded."""
10+
11+
def copy(self) -> "OverflowDict":
12+
"""Return an OverflowDict copy to preserve the overflow marker."""
13+
return OverflowDict(super().copy())
14+
15+
def __copy__(self) -> "OverflowDict":
16+
"""Return an OverflowDict copy to preserve the overflow marker."""
17+
return OverflowDict(super().copy())
18+
19+
def __deepcopy__(self, memo: dict[int, object]) -> "OverflowDict":
20+
"""Return an OverflowDict deepcopy to preserve the overflow marker."""
21+
copied = OverflowDict()
22+
memo[id(self)] = copied
23+
for key, value in self.items():
24+
copied[copy.deepcopy(key, memo)] = copy.deepcopy(value, memo)
25+
return copied

src/qs_codec/utils/utils.py

Lines changed: 134 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,26 @@
2525
from enum import Enum
2626

2727
from ..models.decode_options import DecodeOptions
28+
from ..models.overflow_dict import OverflowDict
2829
from ..models.undefined import Undefined
2930

3031

32+
def _numeric_key_pairs(mapping: t.Mapping[t.Any, t.Any]) -> t.List[t.Tuple[int, t.Any]]:
33+
"""Return (numeric_key, original_key) for keys that coerce to int.
34+
35+
Note: distinct keys like "01" and "1" both coerce to 1; downstream merges
36+
may overwrite earlier values when materializing numeric-keyed dicts.
37+
"""
38+
pairs: t.List[t.Tuple[int, t.Any]] = []
39+
for key in mapping.keys():
40+
try:
41+
numeric_key = int(key)
42+
except (TypeError, ValueError):
43+
continue
44+
pairs.append((numeric_key, key))
45+
return pairs
46+
47+
3148
class Utils:
3249
"""
3350
Namespace container for stateless utility routines.
@@ -143,6 +160,9 @@ def merge(
143160
target = list(target)
144161
target.append(source)
145162
elif isinstance(target, t.Mapping):
163+
if Utils.is_overflow(target):
164+
return Utils.combine(target, source, options)
165+
146166
# Target is a mapping but source is a sequence — coerce indices to string keys.
147167
if isinstance(source, (list, tuple)):
148168
_new = dict(target)
@@ -166,28 +186,58 @@ def merge(
166186
**source,
167187
}
168188

189+
if Utils.is_overflow(source):
190+
source_of = t.cast(OverflowDict, source)
191+
sorted_pairs = sorted(_numeric_key_pairs(source_of), key=lambda item: item[0])
192+
numeric_keys = {key for _, key in sorted_pairs}
193+
result = OverflowDict()
194+
offset = 0
195+
if not isinstance(target, Undefined):
196+
result["0"] = target
197+
offset = 1
198+
for numeric_key, key in sorted_pairs:
199+
val = source_of[key]
200+
if not isinstance(val, Undefined):
201+
# Offset ensures target occupies index "0"; source indices shift up by 1
202+
result[str(numeric_key + offset)] = val
203+
for key, val in source_of.items():
204+
if key in numeric_keys:
205+
continue
206+
if not isinstance(val, Undefined):
207+
result[key] = val
208+
return result
209+
169210
_res: t.List[t.Any] = []
170211
_iter1 = target if isinstance(target, (list, tuple)) else [target]
171212
for _el in _iter1:
172213
if not isinstance(_el, Undefined):
173214
_res.append(_el)
174-
_iter2 = source if isinstance(source, (list, tuple)) else [source]
215+
_iter2 = [source]
175216
for _el in _iter2:
176217
if not isinstance(_el, Undefined):
177218
_res.append(_el)
178219
return _res
179220

180221
# Prepare a mutable copy of the target we can merge into.
222+
is_overflow_target = Utils.is_overflow(target)
181223
merge_target: t.Dict[str, t.Any] = copy.deepcopy(target if isinstance(target, dict) else dict(target))
182224

183225
# For overlapping keys, merge recursively; otherwise, take the new value.
184-
return {
226+
merged_updates: t.Dict[t.Any, t.Any] = {}
227+
# Prefer exact key matches; fall back to string normalization only when needed.
228+
for key, value in source.items():
229+
normalized_key = str(key)
230+
if key in merge_target:
231+
merged_updates[key] = Utils.merge(merge_target[key], value, options)
232+
elif normalized_key in merge_target:
233+
merged_updates[normalized_key] = Utils.merge(merge_target[normalized_key], value, options)
234+
else:
235+
merged_updates[key] = value
236+
merged = {
185237
**merge_target,
186-
**{
187-
str(key): Utils.merge(merge_target[key], value, options) if key in merge_target else value
188-
for key, value in source.items()
189-
},
238+
**merged_updates,
190239
}
240+
return OverflowDict(merged) if is_overflow_target else merged
191241

192242
@staticmethod
193243
def compact(root: t.Dict[str, t.Any]) -> t.Dict[str, t.Any]:
@@ -324,17 +374,90 @@ def _dicts_are_equal(
324374
else:
325375
return d1 == d2
326376

377+
@staticmethod
378+
def is_overflow(obj: t.Any) -> bool:
379+
"""Check if an object is an OverflowDict."""
380+
return isinstance(obj, OverflowDict)
381+
327382
@staticmethod
328383
def combine(
329384
a: t.Union[t.List[t.Any], t.Tuple[t.Any], t.Any],
330385
b: t.Union[t.List[t.Any], t.Tuple[t.Any], t.Any],
331-
) -> t.List[t.Any]:
386+
options: t.Optional[DecodeOptions] = None,
387+
) -> t.Union[t.List[t.Any], t.Dict[str, t.Any]]:
332388
"""
333-
Concatenate two values, treating non‑sequences as singletons.
334-
335-
Returns a new `list`; tuples are expanded but not preserved as tuples.
389+
Concatenate two values, treating non-sequences as singletons.
390+
391+
If `list_limit` is exceeded, converts the list to an `OverflowDict`
392+
(a dict with numeric keys) to prevent memory exhaustion.
393+
When `options` is provided, its ``list_limit`` controls when a list is
394+
converted into an :class:`OverflowDict` (a dict with numeric keys) to
395+
prevent unbounded growth. If ``options`` is ``None``, the default
396+
``list_limit`` from :class:`DecodeOptions` is used.
397+
A negative ``list_limit`` is treated as "overflow immediately": any
398+
non-empty combined result will be converted to :class:`OverflowDict`.
399+
This helper never raises an exception when the limit is exceeded; even
400+
if :class:`DecodeOptions` has ``raise_on_limit_exceeded`` set to
401+
``True``, ``combine`` will still handle overflow only by converting the
402+
list to :class:`OverflowDict`.
336403
"""
337-
return [*(a if isinstance(a, (list, tuple)) else [a]), *(b if isinstance(b, (list, tuple)) else [b])]
404+
if Utils.is_overflow(a):
405+
# a is already an OverflowDict. Append b to a *copy* at the next numeric index.
406+
# We assume sequential keys; len(a_copy) gives the next index.
407+
orig_a = t.cast(OverflowDict, a)
408+
a_copy = OverflowDict({k: v for k, v in orig_a.items() if not isinstance(v, Undefined)})
409+
# Use max key + 1 to handle sparse dicts safely, rather than len(a)
410+
key_pairs = _numeric_key_pairs(a_copy)
411+
idx = (max(key for key, _ in key_pairs) + 1) if key_pairs else 0
412+
413+
if isinstance(b, (list, tuple)):
414+
for item in b:
415+
if not isinstance(item, Undefined):
416+
a_copy[str(idx)] = item
417+
idx += 1
418+
elif Utils.is_overflow(b):
419+
b = t.cast(OverflowDict, b)
420+
# Iterate in numeric key order to preserve list semantics
421+
for _, k in sorted(_numeric_key_pairs(b), key=lambda item: item[0]):
422+
val = b[k]
423+
if not isinstance(val, Undefined):
424+
a_copy[str(idx)] = val
425+
idx += 1
426+
else:
427+
if not isinstance(b, Undefined):
428+
a_copy[str(idx)] = b
429+
return a_copy
430+
431+
# Normal combination: flatten lists/tuples
432+
# Flatten a
433+
if isinstance(a, (list, tuple)):
434+
list_a = [x for x in a if not isinstance(x, Undefined)]
435+
else:
436+
list_a = [a] if not isinstance(a, Undefined) else []
437+
438+
# Flatten b, handling OverflowDict as a list source
439+
if isinstance(b, (list, tuple)):
440+
list_b = [x for x in b if not isinstance(x, Undefined)]
441+
elif Utils.is_overflow(b):
442+
b_of = t.cast(OverflowDict, b)
443+
list_b = [
444+
b_of[k]
445+
for _, k in sorted(_numeric_key_pairs(b_of), key=lambda item: item[0])
446+
if not isinstance(b_of[k], Undefined)
447+
]
448+
else:
449+
list_b = [b] if not isinstance(b, Undefined) else []
450+
451+
res = [*list_a, *list_b]
452+
453+
list_limit = options.list_limit if options else DecodeOptions().list_limit
454+
if list_limit < 0:
455+
return OverflowDict({str(i): x for i, x in enumerate(res)}) if res else res
456+
if len(res) > list_limit:
457+
# Convert to OverflowDict
458+
return OverflowDict({str(i): x for i, x in enumerate(res)})
459+
460+
return res
338461

339462
@staticmethod
340463
def apply(

tests/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.14.0"
8+
"qs": "^6.14.1"
99
}
1010
}

tests/unit/decode_test.py

Lines changed: 58 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from qs_codec import Charset, DecodeOptions, Duplicates, decode, load, loads
1010
from qs_codec.decode import _parse_object
1111
from qs_codec.enums.decode_kind import DecodeKind
12+
from qs_codec.models.overflow_dict import OverflowDict
1213
from qs_codec.utils.decode_utils import DecodeUtils
1314

1415

@@ -318,14 +319,20 @@ def test_parses_an_explicit_list(self, query: str, expected: t.Dict) -> None:
318319
pytest.param("a=b&a[0]=c", None, {"a": ["b", "c"]}, id="simple-first-indexed-list-second"),
319320
pytest.param("a[1]=b&a=c", DecodeOptions(list_limit=20), {"a": ["b", "c"]}, id="indexed-list-with-limit"),
320321
pytest.param(
321-
"a[]=b&a=c", DecodeOptions(list_limit=0), {"a": ["b", "c"]}, id="explicit-list-with-zero-limit"
322+
"a[]=b&a=c",
323+
DecodeOptions(list_limit=0),
324+
{"a": {"0": "b", "1": "c"}},
325+
id="explicit-list-with-zero-limit",
322326
),
323327
pytest.param("a[]=b&a=c", None, {"a": ["b", "c"]}, id="explicit-list-default"),
324328
pytest.param(
325329
"a=b&a[1]=c", DecodeOptions(list_limit=20), {"a": ["b", "c"]}, id="simple-and-indexed-with-limit"
326330
),
327331
pytest.param(
328-
"a=b&a[]=c", DecodeOptions(list_limit=0), {"a": ["b", "c"]}, id="simple-and-explicit-zero-limit"
332+
"a=b&a[]=c",
333+
DecodeOptions(list_limit=0),
334+
{"a": {"0": "b", "1": "c"}},
335+
id="simple-and-explicit-zero-limit",
329336
),
330337
pytest.param("a=b&a[]=c", None, {"a": ["b", "c"]}, id="simple-and-explicit-default"),
331338
],
@@ -574,7 +581,7 @@ def test_parses_lists_of_dicts(self, query: str, expected: t.Mapping[str, t.Any]
574581
pytest.param(
575582
"a[]=b&a[]&a[]=c&a[]=",
576583
DecodeOptions(strict_null_handling=True, list_limit=0),
577-
{"a": ["b", None, "c", ""]},
584+
{"a": {"0": "b", "1": None, "2": "c", "3": ""}},
578585
id="strict-null-and-empty-zero-limit",
579586
),
580587
pytest.param(
@@ -586,7 +593,7 @@ def test_parses_lists_of_dicts(self, query: str, expected: t.Mapping[str, t.Any]
586593
pytest.param(
587594
"a[]=b&a[]=&a[]=c&a[]",
588595
DecodeOptions(strict_null_handling=True, list_limit=0),
589-
{"a": ["b", "", "c", None]},
596+
{"a": {"0": "b", "1": "", "2": "c", "3": None}},
590597
id="empty-and-strict-null-zero-limit",
591598
),
592599
pytest.param("a[]=&a[]=b&a[]=c", None, {"a": ["", "b", "c"]}, id="explicit-empty-first"),
@@ -1328,7 +1335,9 @@ def test_current_list_length_calculation(self) -> None:
13281335
False,
13291336
id="convert-to-map",
13301337
),
1331-
pytest.param("a[]=1&a[]=2", DecodeOptions(list_limit=0), {"a": ["1", "2"]}, False, id="zero-list-limit"),
1338+
pytest.param(
1339+
"a[]=1&a[]=2", DecodeOptions(list_limit=0), {"a": {"0": "1", "1": "2"}}, False, id="zero-list-limit"
1340+
),
13321341
pytest.param(
13331342
"a[]=1&a[]=2",
13341343
DecodeOptions(list_limit=-1, raise_on_limit_exceeded=True),
@@ -1680,3 +1689,47 @@ def test_strict_depth_overflow_raises_for_well_formed(self) -> None:
16801689
def test_unterminated_group_does_not_raise_under_strict_depth(self) -> None:
16811690
segs = DecodeUtils.split_key_into_segments("a[b[c", allow_dots=False, max_depth=5, strict_depth=True)
16821691
assert segs == ["a", "[[b[c]"]
1692+
1693+
1694+
class TestCVE2024:
1695+
def test_dos_attack(self) -> None:
1696+
# JS test:
1697+
# var arr = [];
1698+
# for (var i = 0; i < 105; i++) {
1699+
# arr[arr.length] = 'x';
1700+
# }
1701+
# var attack = 'a[]=' + arr.join('&a[]=');
1702+
# var result = qs.parse(attack, { arrayLimit: 100 });
1703+
# t.notOk(Array.isArray(result.a))
1704+
1705+
arr = ["x"] * 105
1706+
# Construct query: a[]=x&a[]=x...
1707+
attack = "a[]=" + "&a[]=".join(arr)
1708+
1709+
# list_limit is the python equivalent of arrayLimit
1710+
options = DecodeOptions(list_limit=100)
1711+
result = decode(attack, options=options)
1712+
1713+
assert isinstance(result["a"], dict), "Should be a dict when limit exceeded"
1714+
assert isinstance(result["a"], OverflowDict)
1715+
assert len(result["a"]) == 105
1716+
assert result["a"]["0"] == "x"
1717+
assert result["a"]["104"] == "x"
1718+
1719+
def test_array_limit_checks(self) -> None:
1720+
# JS patch tests
1721+
# st.deepEqual(qs.parse('a[]=b', { arrayLimit: 0 }), { a: { 0: 'b' } });
1722+
assert decode("a[]=b", DecodeOptions(list_limit=0)) == {"a": {"0": "b"}}
1723+
1724+
# st.deepEqual(qs.parse('a[]=b&a[]=c', { arrayLimit: 0 }), { a: { 0: 'b', 1: 'c' } });
1725+
assert decode("a[]=b&a[]=c", DecodeOptions(list_limit=0)) == {"a": {"0": "b", "1": "c"}}
1726+
1727+
# st.deepEqual(qs.parse('a[]=b&a[]=c', { arrayLimit: 1 }), { a: { 0: 'b', 1: 'c' } });
1728+
assert decode("a[]=b&a[]=c", DecodeOptions(list_limit=1)) == {"a": {"0": "b", "1": "c"}}
1729+
1730+
# st.deepEqual(qs.parse('a[]=b&a[]=c&a[]=d', { arrayLimit: 2 }), { a: { 0: 'b', 1: 'c', 2: 'd' } });
1731+
assert decode("a[]=b&a[]=c&a[]=d", DecodeOptions(list_limit=2)) == {"a": {"0": "b", "1": "c", "2": "d"}}
1732+
1733+
def test_no_limit_does_not_overflow(self) -> None:
1734+
# Verify that within limit it stays a list
1735+
assert decode("a[]=b&a[]=c", DecodeOptions(list_limit=2)) == {"a": ["b", "c"]}

0 commit comments

Comments
 (0)