Skip to content

Commit 17bbfd0

Browse files
authored
Fix pointer increment issue in _md_shrink causing memory corruption (#1222)
1 parent 863eff1 commit 17bbfd0

File tree

4 files changed

+48
-2
lines changed

4 files changed

+48
-2
lines changed

CHANGES/1221.bugfix.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fixed a memory corruption issue in the C implementation of ``_md_shrink()`` that could lead to segmentation faults and data loss when items were deleted from a :class:`~multidict.MultiDict`. The issue was an edge case in the pointer arithmetic during the compaction phase -- by :user:`bdraco`.

CHANGES/1222.bugfix.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
1221.bugfix.rst

multidict/_multilib/hashtable.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,8 +251,9 @@ _md_shrink(MultiDictObject *md, bool update)
251251
for (Py_ssize_t i = 0; i < nentries; ++i, ++old_ep) {
252252
if (old_ep->identity != NULL) {
253253
if (new_ep != old_ep) {
254-
*new_ep++ = *old_ep;
254+
*new_ep = *old_ep;
255255
}
256+
new_ep++;
256257
} else {
257258
newnentries -= 1;
258259
}

tests/test_mutable_multidict.py

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
from typing import Union
44

55
import pytest
6-
76
from multidict import (
87
CIMultiDict,
98
CIMultiDictProxy,
@@ -872,3 +871,47 @@ def lower(self) -> str:
872871
d.update(lst)
873872

874873
assert [("a", "a2"), ("b", "b"), ("c", "c")] == list(d.items())
874+
875+
876+
def test_multidict_shrink_regression() -> None:
877+
"""
878+
Regression test for _md_shrink pointer increment bug in 6.6.0.
879+
880+
The bug was introduced in PR #1200 which added _md_shrink to optimize
881+
memory usage. The bug occurs when new_ep == old_ep (first non-deleted
882+
entry), causing new_ep to not be incremented. This results in the first
883+
entry being overwritten and memory corruption.
884+
885+
See: https://github.com/aio-libs/multidict/issues/1221
886+
"""
887+
# Test case that reproduces the corruption
888+
md: MultiDict[str] = MultiDict()
889+
890+
# Create pattern: [kept, deleted, kept, kept, ...]
891+
# This triggers new_ep == old_ep on first iteration of _md_shrink
892+
for i in range(10):
893+
md[f"k{i}"] = f"v{i}"
894+
895+
# Delete some entries but keep the first one
896+
# This creates the exact condition for the bug
897+
for i in range(1, 10, 2):
898+
del md[f"k{i}"]
899+
900+
# Trigger shrink by adding many entries
901+
# When the internal array needs to resize, it will call _md_shrink
902+
# because md->used < md->keys->nentries
903+
for i in range(50):
904+
md[f"new{i}"] = f"val{i}"
905+
906+
# The bug would cause k0 to be lost due to memory corruption!
907+
assert "k0" in md, "First entry k0 was lost due to memory corruption!"
908+
assert md["k0"] == "v0", "First entry value was corrupted!"
909+
910+
# Verify all other kept entries survived
911+
for i in range(0, 10, 2):
912+
assert f"k{i}" in md, f"Entry k{i} missing!"
913+
assert md[f"k{i}"] == f"v{i}", f"Entry k{i} has wrong value!"
914+
915+
# Verify new entries
916+
for i in range(50):
917+
assert md[f"new{i}"] == f"val{i}"

0 commit comments

Comments
 (0)