Skip to content

Commit aafb17f

Browse files
committed
Fix exponential expansion based DoS in merge key processing
Resolves #897 Supercedes #916 ### Summary The merge key (`<<`) constructor implementation in `SafeConstructor.flatten_mapping()` was vulnerable to an exponential time and memory complexity Denial of Service (DoS) vulnerability. When mapping/sequence nodes are merged using anchors/aliases, duplicate references to the same alias point to the same MappingNode instance in Python. During merge key processing, the node values are copied and extended in-place. If the same node appears multiple times at different levels, this causes exponential amplification of the elements list: `2^(n+1) - 1`. A small document under 1 KB can trigger millions of element list extensions, exhausting CPU and memory during safe loading. ### Hardened Fix This commit resolves the vulnerability and hardens it against secondary vectors: 1. Tracks node identity using object ID (`id(node)`) in a single `seen` set scoped to the parent mapping's `flatten_mapping()` execution. 2. Checks and skips duplicate node references inside SequenceNode merge keys (resolving PR-916). 3. Checks and skips duplicate node references across separate, independent MappingNode merge keys in the same mapping (e.g., repeating `<<: *anchor` multiple times). 4. Ensures C-based loaders (e.g., `CSafeLoader`, `CLoader`) are also protected since they inherit constructor logic from `SafeConstructor`. ### Performance Impact - Sequence-nested merge duplicates: Loading a 22-level nested document drops from 3.76s to 0.0028s (O(N) linear complexity). - Mapping-level merge duplicates: Loading a 20-level nested document drops from 0.93s to 0.0026s. ### Tests - Added regression tests to `tests/legacy_tests/data/construct-merge.data` and `tests/legacy_tests/data/construct-merge.code` covering both duplicate sequence merges and duplicate direct merges.
1 parent b46710a commit aafb17f

3 files changed

Lines changed: 17 additions & 1 deletion

File tree

lib/yaml/constructor.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,16 +180,19 @@ def construct_scalar(self, node):
180180
def flatten_mapping(self, node):
181181
merge = []
182182
index = 0
183+
seen = set()
183184
while index < len(node.value):
184185
key_node, value_node = node.value[index]
185186
if key_node.tag == 'tag:yaml.org,2002:merge':
186187
del node.value[index]
187188
if isinstance(value_node, MappingNode):
189+
if id(value_node) in seen:
190+
continue
191+
seen.add(id(value_node))
188192
self.flatten_mapping(value_node)
189193
merge.extend(value_node.value)
190194
elif isinstance(value_node, SequenceNode):
191195
submerge = []
192-
seen = set()
193196
for subnode in value_node.value:
194197
if not isinstance(subnode, MappingNode):
195198
raise ConstructorError("while constructing a mapping",

tests/legacy_tests/data/construct-merge.code

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,6 @@
77
{ "x": 1, "y": 2, "r": 10, "label": "center/big" },
88
{ "x": 1, "y": 2, "r": 10, "label": "center/big" },
99
{ "x": 1, "y": 2, "r": 10, "label": "center/big" },
10+
{ "x": 1, "y": 2, "r": 10, "label": "center/big" },
11+
{ "x": 1, "y": 2, "r": 10, "label": "center/big" },
1012
]

tests/legacy_tests/data/construct-merge.data

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,14 @@
2525
<< : [ *BIG, *LEFT, *SMALL ]
2626
x: 1
2727
label: center/big
28+
29+
- # Duplicate sequence merge
30+
<< : [ *CENTER, *CENTER ]
31+
r: 10
32+
label: center/big
33+
34+
- # Duplicate direct merge
35+
<< : *CENTER
36+
<< : *CENTER
37+
r: 10
38+
label: center/big

0 commit comments

Comments
 (0)