From b29f4009bc0028c29574c088dbc4d885256ec33d Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Sat, 3 Oct 2026 06:02:35 +0000 Subject: [PATCH 01/10] Keep an exhausted iterator exhausted after close reader_iter_next checked for a closed reader before it checked whether the iterator had ended. After close(), an exhausted iterator raised ValueError instead of StopIteration, which breaks the iterator protocol. The pure Python iterator, a generator, stops as expected. Mark the iterator as done when next() raises StopIteration, and check that flag first. It needs no lock, because the flag belongs to the iterator. The list of pending records cannot replace the flag: it is already empty when the last record is returned, and the next call must still report a closed or reopened reader, as the pure Python iterator does. Co-Authored-By: Claude Opus 5.5 --- HISTORY.rst | 2 ++ extension/maxminddb.c | 13 +++++++++++++ tests/reader_test.py | 9 +++++++++ 3 files changed, 24 insertions(+) diff --git a/HISTORY.rst b/HISTORY.rst index 84302528..88728dcb 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -32,6 +32,8 @@ History during iteration, from another thread or from a signal handler. * Fixed a crash on free-threaded Python when two threads advanced the same iterator. + * An exhausted iterator now raises ``StopIteration`` after its ``Reader`` + closes, not ``ValueError``. * Added the ``node_byte_size`` and ``search_tree_size`` properties to ``Metadata``, as the pure Python ``Metadata`` has. diff --git a/extension/maxminddb.c b/extension/maxminddb.c index 570878da..c71649a9 100644 --- a/extension/maxminddb.c +++ b/extension/maxminddb.c @@ -88,6 +88,8 @@ typedef struct { Reader_obj *reader; struct record *next; uint64_t generation; + // Set once next() has raised StopIteration. + bool done; } ReaderIter_obj; typedef struct { @@ -915,6 +917,9 @@ static PyObject *ReaderIter_next(PyObject *self) { Py_BEGIN_CRITICAL_SECTION(self); #endif result = reader_iter_next(self); + if (result == NULL && !PyErr_Occurred()) { + ((ReaderIter_obj *)self)->done = true; + } #ifdef Py_GIL_DISABLED Py_END_CRITICAL_SECTION(); #endif @@ -929,6 +934,14 @@ static PyObject *reader_iter_next(PyObject *self) { ReaderIter_obj *ri = (ReaderIter_obj *)self; + // An exhausted iterator stays exhausted, even after the reader closes. + // The list of pending records cannot show this: it is already empty when + // the last record is returned, and the next call must still report a + // closed or reopened reader, as the pure Python iterator does. + if (ri->done) { + return NULL; + } + if (reader_acquire_read_lock(ri->reader) != 0) { return NULL; } diff --git a/tests/reader_test.py b/tests/reader_test.py index 80014bee..e937f09b 100644 --- a/tests/reader_test.py +++ b/tests/reader_test.py @@ -783,6 +783,15 @@ def test_metadata_that_does_not_decode_is_rejected(self) -> None: UnicodeDecodeError, ) + def test_exhausted_iterator_stops_after_close(self) -> None: + with open_database( + f"{_TEST_DATA_DIR}/MaxMind-DB-test-ipv4-24.mmdb", + self.mode, + ) as reader: + iterator = iter(reader) + list(iterator) + self.assertEqual(next(iterator, "done"), "done") + def test_ip_validation(self) -> None: reader = open_database( "tests/data/test-data/MaxMind-DB-test-decoder.mmdb", From 556568d4e90f4b19735bef091828e395452a8e44 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Sat, 3 Oct 2026 05:35:02 +0000 Subject: [PATCH 02/10] Reject a search tree deeper than the address size Neither reader limited the depth of the tree walk during iteration. A corrupt tree, such as one where a node points back to itself, made the C extension set bits past the end of the 16-byte ip_packed array, and then past its heap allocation. The process aborted with heap corruption. The pure Python reader recursed until it raised RecursionError, which a caller that catches InvalidDatabaseError does not catch. A node at the full address depth has no valid children, so both readers now raise InvalidDatabaseError when they reach one. Co-Authored-By: Claude Opus 5.5 --- HISTORY.rst | 4 +++ extension/maxminddb.c | 12 +++++++++ maxminddb/reader.py | 10 +++++--- tests/reader_test.py | 57 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 3 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 88728dcb..4f254930 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -6,6 +6,10 @@ History 3.3.0 ++++++++++++++++++ +* Iterating over a database with a corrupt search tree, such as one with a + cycle, now raises ``InvalidDatabaseError``. Previously, the C extension + could corrupt memory, and the pure Python reader raised ``RecursionError`` + or returned part of the networks. * A second ``__init__`` on a pure Python ``Reader`` now closes the old database, and an iterator from before it raises ``ValueError``, as in the C extension. Before, the iterator walked the new database with node diff --git a/extension/maxminddb.c b/extension/maxminddb.c index c71649a9..8e3ddef0 100644 --- a/extension/maxminddb.c +++ b/extension/maxminddb.c @@ -979,6 +979,18 @@ static PyObject *reader_iter_next(PyObject *self) { // These are aliased networks. Skip them. break; } + // Only a corrupt tree, such as one with a cycle, has a node at + // the full address depth. Without this check, an IPv4 tree + // gives a network longer than /32, and a cycle in either tree + // writes past the end of ip_packed at depth 128. + if (cur->depth >= ri->reader->mmdb->depth) { + reader_release_read_lock(ri->reader); + PyErr_SetString( + state->MaxMindDB_error, + MMDB_strerror(MMDB_CORRUPT_SEARCH_TREE_ERROR)); + free(cur); + return NULL; + } MMDB_search_node_s node; int status = MMDB_read_node( ri->reader->mmdb, (uint32_t)cur->record, &node); diff --git a/maxminddb/reader.py b/maxminddb/reader.py index a310325f..bce07897 100644 --- a/maxminddb/reader.py +++ b/maxminddb/reader.py @@ -29,6 +29,7 @@ _IPV4_MAX_NUM = 2**32 _REOPENED = "Attempt to iterate over a reopened MaxMind DB. Create a new iterator." _CLOSED = "Attempt to iterate over a closed MaxMind DB." +_CORRUPT_TREE = "The MaxMind DB file's search tree is corrupt" class Reader: @@ -241,8 +242,8 @@ def _generate_children(self, node: int, depth: int, ip_acc: int) -> Iterator: return node_count = self._metadata.node_count + bits = 128 if self._metadata.ip_version == 6 else 32 if node > node_count: - bits = 128 if self._metadata.ip_version == 6 else 32 ip_acc <<= bits - depth if ip_acc <= _IPV4_MAX_NUM and bits == 128: depth -= 96 @@ -253,6 +254,10 @@ def _generate_children(self, node: int, depth: int, ip_acc: int) -> Iterator: ), ) elif node < node_count: + # A node at the full address depth has no valid children. Only a + # corrupt tree, such as one with a cycle, has one. + if depth >= bits: + raise InvalidDatabaseError(_CORRUPT_TREE) left = self._read_node(node, 0) ip_acc <<= 1 depth += 1 @@ -310,8 +315,7 @@ def _resolve_data_pointer(self, pointer: int) -> Record: resolved = pointer - self._metadata.node_count + self._metadata.search_tree_size if resolved >= self._buffer_size: - msg = "The MaxMind DB file's search tree is corrupt" - raise InvalidDatabaseError(msg) + raise InvalidDatabaseError(_CORRUPT_TREE) (data, _) = self._decoder.decode(resolved) return data diff --git a/tests/reader_test.py b/tests/reader_test.py index e937f09b..4eeb1a92 100644 --- a/tests/reader_test.py +++ b/tests/reader_test.py @@ -146,6 +146,33 @@ def _database_with_metadata( return data[:start] + _encode_control(7, len(entries)) + items +def _database(records: tuple[int, ...], *, ip_version: int) -> bytes: + """Return a database with a 24-bit search tree and one data record. + + The tree has two records per node, and a record of node_count + 16 points + at the data record, the string "net". + """ + metadata = { + "binary_format_major_version": 2, + "binary_format_minor_version": 0, + "build_epoch": 1, + "database_type": "Test", + "description": {"en": "Test"}, + "ip_version": ip_version, + "languages": ["en"], + "node_count": len(records) // 2, + "record_size": 24, + } + tree = b"".join(record.to_bytes(3, "big") for record in records) + return ( + tree + + bytes(16) + + _encode_value("net") + + _METADATA_START_MARKER + + _encode_value(metadata) + ) + + def _encode_value(value: object, key: str = "") -> bytes: if isinstance(value, str): encoded = value.encode() @@ -792,6 +819,36 @@ def test_exhausted_iterator_stops_after_close(self) -> None: list(iterator) self.assertEqual(next(iterator, "done"), "done") + def test_search_node_at_full_depth_is_rejected(self) -> None: + # Nodes 0 to 32 form a left spine, so node 32 is a search node at + # depth 32, which an IPv4 tree cannot have. + data = 33 + 16 + records = [record for i in range(32) for record in (i + 1, data)] + records += [data, data] + with tempfile.TemporaryDirectory() as directory: + path = pathlib.Path(directory) / "too-deep.mmdb" + path.write_bytes(_database(tuple(records), ip_version=4)) + with ( + open_database(str(path), self.mode) as reader, + self.assertRaisesRegex(InvalidDatabaseError, "search tree is corrupt"), + ): + list(reader) + + def test_cyclic_search_tree_is_rejected(self) -> None: + data = bytearray( + pathlib.Path(f"{_TEST_DATA_DIR}/MaxMind-DB-test-ipv4-24.mmdb").read_bytes(), + ) + # Point the left record of node 1 back at node 1. + data[6:9] = b"\x00\x00\x01" + with tempfile.TemporaryDirectory() as directory: + path = pathlib.Path(directory) / "cyclic.mmdb" + path.write_bytes(data) + with ( + open_database(str(path), self.mode) as reader, + self.assertRaisesRegex(InvalidDatabaseError, "search tree is corrupt"), + ): + list(reader) + def test_ip_validation(self) -> None: reader = open_database( "tests/data/test-data/MaxMind-DB-test-decoder.mmdb", From 61b1f97cf7029ba33d46185295d8b690b76bad85 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 6 Oct 2026 15:05:40 +0000 Subject: [PATCH 03/10] Stop an iterator after an error After an error, such as a corrupt search tree, the C iterator kept its pending records, and the next call continued from them. With a cycle in the tree, a caller that skipped bad records could get many errors before StopIteration. The pure Python iterator is a generator, so it stops after its first error. Free the pending records and mark the iterator done when next() fails, so the C iterator stops too. Reuse the loop from ReaderIter_dealloc as free_records. Co-Authored-By: Claude Opus 5.5 --- HISTORY.rst | 3 +++ extension/maxminddb.c | 35 ++++++++++++++++++++++------------- tests/reader_test.py | 17 ++++++++++++----- 3 files changed, 37 insertions(+), 18 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 4f254930..cd1e28c7 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -38,6 +38,9 @@ History iterator. * An exhausted iterator now raises ``StopIteration`` after its ``Reader`` closes, not ``ValueError``. + * The iterator now stops after any error, as the pure Python iterator + does. This includes an error in the data of one record. Previously, the + next call returned the remaining networks. * Added the ``node_byte_size`` and ``search_tree_size`` properties to ``Metadata``, as the pure Python ``Metadata`` has. diff --git a/extension/maxminddb.c b/extension/maxminddb.c index 8e3ddef0..9b39f85f 100644 --- a/extension/maxminddb.c +++ b/extension/maxminddb.c @@ -88,7 +88,7 @@ typedef struct { Reader_obj *reader; struct record *next; uint64_t generation; - // Set once next() has raised StopIteration. + // Set once next() has raised StopIteration or an error. bool done; } ReaderIter_obj; @@ -141,6 +141,7 @@ static PyObject *metadata_value(PyObject *map, const char *key); static void set_error_from_cause(PyObject *type, const char *message); static PyObject *Metadata_node_byte_size(PyObject *self, void *closure); static PyObject *reader_iter_next(PyObject *self); +static void free_records(struct record *next); static bool format_sockaddr(struct sockaddr *addr, char *dst); static PyObject *from_entry_data_list(maxminddb_state *state, MMDB_entry_data_list_s **entry_data_list); @@ -917,8 +918,12 @@ static PyObject *ReaderIter_next(PyObject *self) { Py_BEGIN_CRITICAL_SECTION(self); #endif result = reader_iter_next(self); - if (result == NULL && !PyErr_Occurred()) { - ((ReaderIter_obj *)self)->done = true; + // Stop after StopIteration or an error, as a generator does. + if (result == NULL) { + ReaderIter_obj *ri = (ReaderIter_obj *)self; + free_records(ri->next); + ri->next = NULL; + ri->done = true; } #ifdef Py_GIL_DISABLED Py_END_CRITICAL_SECTION(); @@ -934,10 +939,11 @@ static PyObject *reader_iter_next(PyObject *self) { ReaderIter_obj *ri = (ReaderIter_obj *)self; - // An exhausted iterator stays exhausted, even after the reader closes. - // The list of pending records cannot show this: it is already empty when - // the last record is returned, and the next call must still report a - // closed or reopened reader, as the pure Python iterator does. + // An iterator that is exhausted or that raised an error stays done, even + // after the reader closes. The list of pending records cannot show this: + // it is already empty when the last record is returned, and the next call + // must still report a closed or reopened reader, as the pure Python + // iterator does. if (ri->done) { return NULL; } @@ -1126,17 +1132,20 @@ static PyObject *reader_iter_next(PyObject *self) { return NULL; } -static void ReaderIter_dealloc(PyObject *self) { - ReaderIter_obj *ri = (ReaderIter_obj *)self; - - Py_DECREF(ri->reader); - - struct record *next = ri->next; +static void free_records(struct record *next) { while (next != NULL) { struct record *cur = next; next = cur->next; free(cur); } +} + +static void ReaderIter_dealloc(PyObject *self) { + ReaderIter_obj *ri = (ReaderIter_obj *)self; + + Py_DECREF(ri->reader); + + free_records(ri->next); PyTypeObject *type = Py_TYPE(self); PyObject_Del(self); Py_DECREF(type); diff --git a/tests/reader_test.py b/tests/reader_test.py index 4eeb1a92..9669262c 100644 --- a/tests/reader_test.py +++ b/tests/reader_test.py @@ -843,11 +843,15 @@ def test_cyclic_search_tree_is_rejected(self) -> None: with tempfile.TemporaryDirectory() as directory: path = pathlib.Path(directory) / "cyclic.mmdb" path.write_bytes(data) - with ( - open_database(str(path), self.mode) as reader, - self.assertRaisesRegex(InvalidDatabaseError, "search tree is corrupt"), - ): - list(reader) + with open_database(str(path), self.mode) as reader: + iterator = iter(reader) + with self.assertRaisesRegex( + InvalidDatabaseError, + "search tree is corrupt", + ): + list(iterator) + # The iterator stops after an error, as a generator does. + self.assertEqual(next(iterator, "done"), "done") def test_ip_validation(self) -> None: reader = open_database( @@ -1547,6 +1551,9 @@ def ip_network(*args, **kwargs): pass else: sys.exit("next() after close() did not raise ValueError") + # The error stops the iterator. + if next(iterator, None) is not None: + sys.exit("the iterator continued after the error") print("ok") """, ) From bf06c09b6cb062154dfd19bc612c68e25fd7c3d4 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Sat, 3 Oct 2026 06:47:31 +0000 Subject: [PATCH 04/10] Convert only IPv4 networks when iterating an IPv6 tree Both iterators turned any network whose first 96 bits were zero into an IPv4 network and subtracted 96 from its prefix length. For a network shorter than /96, such as ::/1, the prefix length became negative, and iteration raised ValueError. An IPv4 network in an IPv6 tree is at least /96, so convert only those. The pure Python iterator had two more bugs here. It compared the address with 2**32 using <=, so ::1:0:0/96 got a prefix length of 0 and raised ValueError. It also skipped every data record equal to the IPv4 start node, although only a search node can be the IPv4 subtree. Use <, and skip only a search node, as the C iterator does. Build the network with IPv4Network or IPv6Network, because ip_network() picks IPv4 for any small integer. Its alias rule also differed from the C iterator. It skipped the IPv4 start node in an IPv4 tree, where a record that points back to the root is a cycle, and inside the IPv4 subtree of an IPv6 tree. Both hid a corrupt tree behind partial results. Skip the subtree only when an address with a set bit in its first 96 bits leads to it, as the C iterator does, and raise InvalidDatabaseError for a record that points to the root, which libmaxminddb treats as invalid. Co-Authored-By: Claude Opus 5.5 --- HISTORY.rst | 5 +++ extension/maxminddb.c | 9 ++++- maxminddb/reader.py | 41 ++++++++++++-------- tests/reader_test.py | 89 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 126 insertions(+), 18 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index cd1e28c7..15cac079 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -6,6 +6,11 @@ History 3.3.0 ++++++++++++++++++ +* Fixed iteration over an IPv6 database with a network shorter than /96 + whose first bits are zero, such as ``::/1``. The readers raised + ``ValueError`` or skipped networks. The pure Python reader also raised + ``ValueError`` for a network that starts at ``::1:0:0``, such as + ``::1:0:0/96``. * Iterating over a database with a corrupt search tree, such as one with a cycle, now raises ``InvalidDatabaseError``. Previously, the C extension could corrupt memory, and the pure Python reader raised ``RecursionError`` diff --git a/extension/maxminddb.c b/extension/maxminddb.c index 9b39f85f..ffa69d3a 100644 --- a/extension/maxminddb.c +++ b/extension/maxminddb.c @@ -974,8 +974,11 @@ static PyObject *reader_iter_next(PyObject *self) { switch (cur->type) { case MMDB_RECORD_TYPE_INVALID: reader_release_read_lock(ri->reader); + // libmaxminddb before 1.14 returns this type for a record + // that points to the root or past the data section. Later + // versions fail in MMDB_read_node instead. PyErr_SetString(state->MaxMindDB_error, - "Invalid record when reading node"); + MMDB_strerror(MMDB_CORRUPT_SEARCH_TREE_ERROR)); free(cur); return NULL; case MMDB_RECORD_TYPE_SEARCH_NODE: { @@ -1081,7 +1084,9 @@ static PyObject *reader_iter_next(PyObject *self) { int ip_start = 0; Py_ssize_t ip_length = 4; if (depth == 128) { - if (is_ipv6(cur->ip_packed)) { + // A network shorter than /96 is IPv6, even if its first + // 96 bits are zero. + if (is_ipv6(cur->ip_packed) || cur->depth < 96) { // IPv6 address ip_length = 16; } else { diff --git a/maxminddb/reader.py b/maxminddb/reader.py index bce07897..d74965c7 100644 --- a/maxminddb/reader.py +++ b/maxminddb/reader.py @@ -10,7 +10,7 @@ import contextlib import ipaddress from dataclasses import dataclass -from ipaddress import IPv4Address, IPv6Address +from ipaddress import IPv4Address, IPv4Network, IPv6Address, IPv6Network from typing import IO, TYPE_CHECKING, Any from maxminddb.const import MODE_AUTO, MODE_FD, MODE_FILE, MODE_MEMORY, MODE_MMAP @@ -237,26 +237,35 @@ def _iterate(self, generation: int) -> Iterator: yield record def _generate_children(self, node: int, depth: int, ip_acc: int) -> Iterator: - if ip_acc != 0 and node == self._ipv4_start: - # Skip nodes aliased to IPv4 - return - node_count = self._metadata.node_count bits = 128 if self._metadata.ip_version == 6 else 32 if node > node_count: ip_acc <<= bits - depth - if ip_acc <= _IPV4_MAX_NUM and bits == 128: - depth -= 96 - yield ( - ipaddress.ip_network((ip_acc, depth)), - self._resolve_data_pointer( - node, - ), - ) + network: IPv4Network | IPv6Network + if bits == 32: + network = IPv4Network((ip_acc, depth)) + elif depth >= 96 and ip_acc < _IPV4_MAX_NUM: + # An IPv4 network in an IPv6 tree is at least /96, and its + # first 96 bits are zero. + network = IPv4Network((ip_acc, depth - 96)) + else: + network = IPv6Network((ip_acc, depth)) + yield (network, self._resolve_data_pointer(node)) elif node < node_count: - # A node at the full address depth has no valid children. Only a - # corrupt tree, such as one with a cycle, has one. - if depth >= bits: + # Skip the IPv4 subtree when an address with a set bit in its first + # 96 bits leads to it, as the C extension does. Inside the IPv4 + # subtree, or in an IPv4 tree, a record that points back to it is a + # cycle. + if ( + node == self._ipv4_start + and bits == 128 + and ip_acc >> max(depth - 96, 0) != 0 + ): + return + # A node at the full address depth has no valid children, and no + # record can point to the root. Only a corrupt tree, such as one + # with a cycle, has either. + if depth >= bits or (node == 0 and depth > 0): raise InvalidDatabaseError(_CORRUPT_TREE) left = self._read_node(node, 0) ip_acc <<= 1 diff --git a/tests/reader_test.py b/tests/reader_test.py index 9669262c..6a401f65 100644 --- a/tests/reader_test.py +++ b/tests/reader_test.py @@ -834,6 +834,62 @@ def test_search_node_at_full_depth_is_rejected(self) -> None: ): list(reader) + def test_record_that_points_to_the_root_is_rejected(self) -> None: + # Node 1's right record points back to the root. The left records point + # at data, so a walk through the root again would yield networks that + # the tree does not have, such as 192.0.0.0/3. + records = (18, 1, 18, 0) + with tempfile.TemporaryDirectory() as directory: + path = pathlib.Path(directory) / "root-record.mmdb" + path.write_bytes(_database(records, ip_version=4)) + with open_database(str(path), self.mode) as reader: + seen: list[ipaddress.IPv4Network | ipaddress.IPv6Network] = [] + with self.assertRaisesRegex( + InvalidDatabaseError, + "search tree is corrupt", + ): + for network, _ in reader: + seen.append(network) + valid = { + ipaddress.ip_network("0.0.0.0/1"), + ipaddress.ip_network("128.0.0.0/2"), + } + self.assertLessEqual(set(seen), valid) + # Both readers yield node 0's left record before node 1. + self.assertIn(ipaddress.ip_network("0.0.0.0/1"), seen) + + def test_iterate_ipv6_networks_shorter_than_96_bits(self) -> None: + # One node whose two records point at the same data record, so the + # tree holds ::/1 and 8000::/1. + records = (17, 17) + with tempfile.TemporaryDirectory() as directory: + path = pathlib.Path(directory) / "short-ipv6.mmdb" + path.write_bytes(_database(records, ip_version=6)) + with open_database(str(path), self.mode) as reader: + self.assertEqual( + list(reader), + [ + (ipaddress.ip_network("::/1"), "net"), + (ipaddress.ip_network("8000::/1"), "net"), + ], + ) + + def test_iterate_ipv6_network_just_above_ipv4(self) -> None: + # A left spine of 96 nodes whose last right record is data holds only + # ::1:0:0/96. The first 96 bits are not all zero, so it is IPv6. + empty = 96 + data = empty + 16 + records = [record for i in range(95) for record in (i + 1, empty)] + records += [empty, data] + with tempfile.TemporaryDirectory() as directory: + path = pathlib.Path(directory) / "ipv6-above-ipv4.mmdb" + path.write_bytes(_database(tuple(records), ip_version=6)) + with open_database(str(path), self.mode) as reader: + self.assertEqual( + [network for network, _ in reader], + [ipaddress.ip_network("::1:0:0/96")], + ) + def test_cyclic_search_tree_is_rejected(self) -> None: data = bytearray( pathlib.Path(f"{_TEST_DATA_DIR}/MaxMind-DB-test-ipv4-24.mmdb").read_bytes(), @@ -853,6 +909,39 @@ def test_cyclic_search_tree_is_rejected(self) -> None: # The iterator stops after an error, as a generator does. self.assertEqual(next(iterator, "done"), "done") + # A record that points back to the root of an IPv4 tree. + broken = f"{_TEST_DATA_DIR}/MaxMind-DB-test-broken-search-tree-24.mmdb" + with ( + open_database(broken, self.mode) as reader, + self.assertRaisesRegex(InvalidDatabaseError, "search tree is corrupt"), + ): + list(reader) + + # A record in the IPv4 subtree of an IPv6 tree that points back to the + # IPv4 start node. Each record of that node, ::/97 and ::8000:0/97, + # points to it in turn. The right record makes the address nonzero. + source = f"{_TEST_DATA_DIR}/MaxMind-DB-test-mixed-24.mmdb" + with maxminddb.reader.Reader(source) as reader: + ipv4_start = reader._ipv4_start # noqa: SLF001 + original = pathlib.Path(source).read_bytes() + for index in (0, 1): + offset = ipv4_start * 6 + index * 3 + mixed = bytearray(original) + mixed[offset : offset + 3] = ipv4_start.to_bytes(3, "big") + with ( + self.subTest(index=index), + tempfile.TemporaryDirectory() as directory, + ): + path = pathlib.Path(directory) / "ipv4-cycle.mmdb" + path.write_bytes(mixed) + with ( + open_database(str(path), self.mode) as reader, + self.assertRaisesRegex( + InvalidDatabaseError, "search tree is corrupt" + ), + ): + list(reader) + def test_ip_validation(self) -> None: reader = open_database( "tests/data/test-data/MaxMind-DB-test-decoder.mmdb", From 2989384652cc8c4853114da0a4fec6a8f92d009f Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Sat, 3 Oct 2026 12:30:35 +0000 Subject: [PATCH 05/10] Reject a data pointer into the separator _resolve_data_pointer checked only that a data pointer was inside the buffer. A search tree record that pointed into the 16-byte separator between the search tree and the data section decoded the zero bytes there, so get() returned {} and iteration yielded the network with {}. libmaxminddb rejects such a record as a corrupt search tree. Reject a pointer before the start of the data section too. A lookup benchmark showed no measurable cost. Co-Authored-By: Claude Opus 5.5 --- HISTORY.rst | 3 +++ maxminddb/reader.py | 24 ++++++++++++++---------- tests/reader_test.py | 16 ++++++++++++++++ 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 15cac079..28169687 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -11,6 +11,9 @@ History ``ValueError`` or skipped networks. The pure Python reader also raised ``ValueError`` for a network that starts at ``::1:0:0``, such as ``::1:0:0/96``. +* The pure Python reader now raises ``InvalidDatabaseError`` for a search tree + record that points before the data section. Previously, it returned an + empty map. * Iterating over a database with a corrupt search tree, such as one with a cycle, now raises ``InvalidDatabaseError``. Previously, the C extension could corrupt memory, and the pure Python reader raised ``RecursionError`` diff --git a/maxminddb/reader.py b/maxminddb/reader.py index d74965c7..5ad74431 100644 --- a/maxminddb/reader.py +++ b/maxminddb/reader.py @@ -49,6 +49,8 @@ class Reader: _metadata: Metadata _record_size: int _ipv4_start: int + _search_tree_size: int + _data_start: int # Incremented on each open, so an iterator can detect a reopen. _generation: int = 0 @@ -134,22 +136,22 @@ def _load( self._metadata = Metadata(**_metadata_fields(metadata, filename)) self._record_size = self._metadata.record_size + # _resolve_data_pointer uses these on every lookup. + self._search_tree_size = self._metadata.search_tree_size + self._data_start = ( + self._search_tree_size + self._DATA_SECTION_SEPARATOR_SIZE + ) + # Traversal reads nodes below node_count. Once the tree fits, those # reads need no length checks of their own. - tree_end = ( - self._metadata.search_tree_size + self._DATA_SECTION_SEPARATOR_SIZE - ) - if tree_end > self._buffer_size: + if self._data_start > self._buffer_size: msg = ( f"Error opening database file ({filename}). The search tree " "extends past the end of the file." ) raise InvalidDatabaseError(msg) # noqa: TRY301 - self._decoder = Decoder( - self._buffer, - self._metadata.search_tree_size + self._DATA_SECTION_SEPARATOR_SIZE, - ) + self._decoder = Decoder(self._buffer, self._data_start) self.closed = False ipv4_start = 0 @@ -321,9 +323,11 @@ def _read_node(self, node_number: int, index: int) -> int: raise InvalidDatabaseError(msg) def _resolve_data_pointer(self, pointer: int) -> Record: - resolved = pointer - self._metadata.node_count + self._metadata.search_tree_size + resolved = pointer - self._metadata.node_count + self._search_tree_size - if resolved >= self._buffer_size: + # A pointer into the separator between the tree and the data section + # is as corrupt as one past the end, as libmaxminddb checks. + if resolved < self._data_start or resolved >= self._buffer_size: raise InvalidDatabaseError(_CORRUPT_TREE) (data, _) = self._decoder.decode(resolved) diff --git a/tests/reader_test.py b/tests/reader_test.py index 6a401f65..45db4876 100644 --- a/tests/reader_test.py +++ b/tests/reader_test.py @@ -890,6 +890,22 @@ def test_iterate_ipv6_network_just_above_ipv4(self) -> None: [ipaddress.ip_network("::1:0:0/96")], ) + def test_record_that_points_into_the_separator_is_rejected(self) -> None: + # The left record, node_count + 1, points into the 16-byte separator + # between the search tree and the data section. libmaxminddb before + # 1.14 reports it as bad data. + with tempfile.TemporaryDirectory() as directory: + path = pathlib.Path(directory) / "separator.mmdb" + path.write_bytes(_database((2, 17), ip_version=4)) + with ( + open_database(str(path), self.mode) as reader, + self.assertRaisesRegex( + InvalidDatabaseError, + "search tree is corrupt|contains bad data", + ), + ): + reader.get(self.ipf("1.1.1.1")) + def test_cyclic_search_tree_is_rejected(self) -> None: data = bytearray( pathlib.Path(f"{_TEST_DATA_DIR}/MaxMind-DB-test-ipv4-24.mmdb").read_bytes(), From e63c8a8b3a62cbab402ccb484bfbbf535e9d363c Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 13 Oct 2026 16:42:56 +0000 Subject: [PATCH 06/10] Keep the C iterator usable after a read lock failure The iterator freed its pending records and stopped after any error, including a failure to get the read lock. That failure is temporary and happens before the iterator removes a record, so a caller that caught the RuntimeError and called next() again got StopIteration and lost the rest of the database. Get the lock in ReaderIter_next, and stop only after an error from reader_iter_next, which now runs with the lock held. Co-Authored-By: Claude Opus 5.5 --- HISTORY.rst | 7 ++++--- extension/maxminddb.c | 43 ++++++++++++++++++++----------------------- 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 28169687..be72a238 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -46,9 +46,10 @@ History iterator. * An exhausted iterator now raises ``StopIteration`` after its ``Reader`` closes, not ``ValueError``. - * The iterator now stops after any error, as the pure Python iterator - does. This includes an error in the data of one record. Previously, the - next call returned the remaining networks. + * The iterator now stops after an error, as the pure Python iterator does. + This includes an error in the data of one record, but not a failure to + get the read lock. Previously, the next call returned the remaining + networks. * Added the ``node_byte_size`` and ``search_tree_size`` properties to ``Metadata``, as the pure Python ``Metadata`` has. diff --git a/extension/maxminddb.c b/extension/maxminddb.c index ffa69d3a..f8d99bfd 100644 --- a/extension/maxminddb.c +++ b/extension/maxminddb.c @@ -910,20 +910,28 @@ static bool is_ipv6(char ip[16]) { } static PyObject *ReaderIter_next(PyObject *self) { - PyObject *result; + ReaderIter_obj *ri = (ReaderIter_obj *)self; + PyObject *result = NULL; #ifdef Py_GIL_DISABLED // The iterator's list of pending records is not thread-safe, so let only // one thread at a time advance an iterator. The read lock is shared, so // it does not do this. Py_BEGIN_CRITICAL_SECTION(self); #endif - result = reader_iter_next(self); - // Stop after StopIteration or an error, as a generator does. - if (result == NULL) { - ReaderIter_obj *ri = (ReaderIter_obj *)self; - free_records(ri->next); - ri->next = NULL; - ri->done = true; + // An iterator that is exhausted or that raised an error stays done, even + // after the reader closes. The list of pending records cannot show this: + // it is already empty when the last record is returned, and the next call + // must still report a closed or reopened reader, as the pure Python + // iterator does. A failure to get the read lock is temporary and removes + // no record, so the iterator can continue after it. + if (!ri->done && reader_acquire_read_lock(ri->reader) == 0) { + result = reader_iter_next(self); + // Stop after StopIteration or another error, as a generator does. + if (result == NULL) { + free_records(ri->next); + ri->next = NULL; + ri->done = true; + } } #ifdef Py_GIL_DISABLED Py_END_CRITICAL_SECTION(); @@ -931,24 +939,13 @@ static PyObject *ReaderIter_next(PyObject *self) { return result; } +// The caller holds the read lock. This function releases it on every path. static PyObject *reader_iter_next(PyObject *self) { - maxminddb_state *state = get_maxminddb_state_from_self((PyObject *)self); - if (state == NULL) { - return NULL; - } - ReaderIter_obj *ri = (ReaderIter_obj *)self; - // An iterator that is exhausted or that raised an error stays done, even - // after the reader closes. The list of pending records cannot show this: - // it is already empty when the last record is returned, and the next call - // must still report a closed or reopened reader, as the pure Python - // iterator does. - if (ri->done) { - return NULL; - } - - if (reader_acquire_read_lock(ri->reader) != 0) { + maxminddb_state *state = get_maxminddb_state_from_self((PyObject *)self); + if (state == NULL) { + reader_release_read_lock(ri->reader); return NULL; } From 4229d09696bbfb35daa9f5ea80301556692ed87d Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 13 Oct 2026 16:43:24 +0000 Subject: [PATCH 07/10] Share the corrupt-tree exit of the C iterator The MMDB_RECORD_TYPE_INVALID case and the depth check repeated the same error block. Move it into corrupt_tree_error, so that the two cannot drift apart. Co-Authored-By: Claude Opus 5.5 --- extension/maxminddb.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/extension/maxminddb.c b/extension/maxminddb.c index f8d99bfd..f95f18d3 100644 --- a/extension/maxminddb.c +++ b/extension/maxminddb.c @@ -141,6 +141,9 @@ static PyObject *metadata_value(PyObject *map, const char *key); static void set_error_from_cause(PyObject *type, const char *message); static PyObject *Metadata_node_byte_size(PyObject *self, void *closure); static PyObject *reader_iter_next(PyObject *self); +static PyObject *corrupt_tree_error(ReaderIter_obj *ri, + maxminddb_state *state, + struct record *cur); static void free_records(struct record *next); static bool format_sockaddr(struct sockaddr *addr, char *dst); static PyObject *from_entry_data_list(maxminddb_state *state, @@ -970,14 +973,10 @@ static PyObject *reader_iter_next(PyObject *self) { switch (cur->type) { case MMDB_RECORD_TYPE_INVALID: - reader_release_read_lock(ri->reader); // libmaxminddb before 1.14 returns this type for a record // that points to the root or past the data section. Later // versions fail in MMDB_read_node instead. - PyErr_SetString(state->MaxMindDB_error, - MMDB_strerror(MMDB_CORRUPT_SEARCH_TREE_ERROR)); - free(cur); - return NULL; + return corrupt_tree_error(ri, state, cur); case MMDB_RECORD_TYPE_SEARCH_NODE: { if (cur->record == ri->reader->mmdb->ipv4_start_node.node_value && @@ -990,12 +989,7 @@ static PyObject *reader_iter_next(PyObject *self) { // gives a network longer than /32, and a cycle in either tree // writes past the end of ip_packed at depth 128. if (cur->depth >= ri->reader->mmdb->depth) { - reader_release_read_lock(ri->reader); - PyErr_SetString( - state->MaxMindDB_error, - MMDB_strerror(MMDB_CORRUPT_SEARCH_TREE_ERROR)); - free(cur); - return NULL; + return corrupt_tree_error(ri, state, cur); } MMDB_search_node_s node; int status = MMDB_read_node( @@ -1134,6 +1128,17 @@ static PyObject *reader_iter_next(PyObject *self) { return NULL; } +// Sets the corrupt-tree error, releases the read lock and frees cur. +static PyObject *corrupt_tree_error(ReaderIter_obj *ri, + maxminddb_state *state, + struct record *cur) { + reader_release_read_lock(ri->reader); + PyErr_SetString(state->MaxMindDB_error, + MMDB_strerror(MMDB_CORRUPT_SEARCH_TREE_ERROR)); + free(cur); + return NULL; +} + static void free_records(struct record *next) { while (next != NULL) { struct record *cur = next; From ec347d902a18543a1f5776418bd5634790fb4eed Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 13 Oct 2026 16:43:44 +0000 Subject: [PATCH 08/10] Move free_records below ReaderIter_dealloc Put the helper after its callers. This commit only moves code. Co-Authored-By: Claude Opus 5.5 --- extension/maxminddb.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/extension/maxminddb.c b/extension/maxminddb.c index f95f18d3..de5aa0ca 100644 --- a/extension/maxminddb.c +++ b/extension/maxminddb.c @@ -1139,14 +1139,6 @@ static PyObject *corrupt_tree_error(ReaderIter_obj *ri, return NULL; } -static void free_records(struct record *next) { - while (next != NULL) { - struct record *cur = next; - next = cur->next; - free(cur); - } -} - static void ReaderIter_dealloc(PyObject *self) { ReaderIter_obj *ri = (ReaderIter_obj *)self; @@ -1158,6 +1150,14 @@ static void ReaderIter_dealloc(PyObject *self) { Py_DECREF(type); } +static void free_records(struct record *next) { + while (next != NULL) { + struct record *cur = next; + next = cur->next; + free(cur); + } +} + // Metadata is immutable, so tp_new sets every field and there is no tp_init. // No object can then have a NULL field or be initialized twice. static PyObject * From c81339a55492499dd46e24575cd549a5e18d9153 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 13 Oct 2026 16:44:01 +0000 Subject: [PATCH 09/10] Store one offset for data pointers _resolve_data_pointer subtracted node_count and added the search tree size on every lookup. Store the difference once, in _pointer_base, in place of _search_tree_size. Co-Authored-By: Claude Opus 5.5 --- maxminddb/reader.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/maxminddb/reader.py b/maxminddb/reader.py index 5ad74431..c27b3286 100644 --- a/maxminddb/reader.py +++ b/maxminddb/reader.py @@ -49,7 +49,7 @@ class Reader: _metadata: Metadata _record_size: int _ipv4_start: int - _search_tree_size: int + _pointer_base: int _data_start: int # Incremented on each open, so an iterator can detect a reopen. _generation: int = 0 @@ -136,11 +136,11 @@ def _load( self._metadata = Metadata(**_metadata_fields(metadata, filename)) self._record_size = self._metadata.record_size - # _resolve_data_pointer uses these on every lookup. - self._search_tree_size = self._metadata.search_tree_size - self._data_start = ( - self._search_tree_size + self._DATA_SECTION_SEPARATOR_SIZE - ) + # _resolve_data_pointer uses these on every lookup. A data record + # minus node_count is an offset from the end of the search tree. + search_tree_size = self._metadata.search_tree_size + self._pointer_base = search_tree_size - self._metadata.node_count + self._data_start = search_tree_size + self._DATA_SECTION_SEPARATOR_SIZE # Traversal reads nodes below node_count. Once the tree fits, those # reads need no length checks of their own. @@ -323,7 +323,7 @@ def _read_node(self, node_number: int, index: int) -> int: raise InvalidDatabaseError(msg) def _resolve_data_pointer(self, pointer: int) -> Record: - resolved = pointer - self._metadata.node_count + self._search_tree_size + resolved = pointer + self._pointer_base # A pointer into the separator between the tree and the data section # is as corrupt as one past the end, as libmaxminddb checks. From ba551ef623d6b101598a414aed63fd33eef97078 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 8 Oct 2026 20:58:04 +0000 Subject: [PATCH 10/10] Compute the walk constants once per reader _generate_children read node_count and computed the address size from the metadata on every call, which is about twice per node in the tree. Both are constant for a reader, so set them in _load, as _ipv4_start is. With data decoding stubbed out, this made a walk of GeoLite City about 1% faster. Co-Authored-By: Claude Opus 5.5 --- maxminddb/reader.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/maxminddb/reader.py b/maxminddb/reader.py index c27b3286..ac2b71ff 100644 --- a/maxminddb/reader.py +++ b/maxminddb/reader.py @@ -49,6 +49,8 @@ class Reader: _metadata: Metadata _record_size: int _ipv4_start: int + _node_count: int + _address_bits: int _pointer_base: int _data_start: int # Incremented on each open, so an iterator can detect a reopen. @@ -166,6 +168,13 @@ def _load( node = self._read_node(node, 0) ipv4_start = node self._ipv4_start = ipv4_start + + # _generate_children uses these on every node. + self._node_count = self._metadata.node_count + if self._metadata.ip_version == 6: + self._address_bits = 128 + else: + self._address_bits = 32 except BaseException: _close_buffer(self.__dict__.get("_buffer")) raise @@ -239,8 +248,8 @@ def _iterate(self, generation: int) -> Iterator: yield record def _generate_children(self, node: int, depth: int, ip_acc: int) -> Iterator: - node_count = self._metadata.node_count - bits = 128 if self._metadata.ip_version == 6 else 32 + node_count = self._node_count + bits = self._address_bits if node > node_count: ip_acc <<= bits - depth network: IPv4Network | IPv6Network