From cd4496b1447ecb6cbae68392edfaf06d63f1beda Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 30 Oct 2026 20:04:35 +0000 Subject: [PATCH 1/6] Reject extended type byte 0 in pure Python reader The check for an invalid extended type byte compared the type number to 7, so it never matched. Extended type byte 0 decoded as a map. It now raises InvalidDatabaseError. The C extension still decodes this byte as a map, because libmaxminddb does not check it. Co-Authored-By: Claude Opus 5.5 --- HISTORY.rst | 3 +++ maxminddb/decoder.py | 13 +++---------- tests/decoder_test.py | 10 ++++++++++ 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 755aba2..234b105 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -30,6 +30,9 @@ History * The pure Python reader now raises ``InvalidDatabaseError`` when a record map key is not a string, as the C extension does. Previously, it returned the non-string key or raised ``TypeError``. +* The pure Python reader now raises ``InvalidDatabaseError`` for extended type + byte 0. Previously, it decoded the value as a map. The C extension still + decodes it as a map, because libmaxminddb does not check this byte. * An empty database file now raises ``InvalidDatabaseError`` in ``MODE_FILE``, as in ``MODE_MEMORY``. Previously, it raised ``OSError`` on platforms with ``os.pread``. diff --git a/maxminddb/decoder.py b/maxminddb/decoder.py index 575eebe..65a1351 100644 --- a/maxminddb/decoder.py +++ b/maxminddb/decoder.py @@ -399,16 +399,9 @@ def _decode( # noqa: C901, PLR0911, PLR0912 def _read_extended(self, offset: int) -> tuple[int, int]: next_byte = self._buffer[offset] - type_num = next_byte + 7 - if type_num < 7: - msg = ( - "Something went horribly wrong in the decoder. An " - f"extended type resolved to a type number < 8 ({type_num})" - ) - raise InvalidDatabaseError( - msg, - ) - return type_num, offset + 1 + if not next_byte: + raise InvalidDatabaseError(_BAD_DATA) + return next_byte + 7, offset + 1 @staticmethod def _verify_size(expected: int, actual: int) -> None: diff --git a/tests/decoder_test.py b/tests/decoder_test.py index c2820a3..3dd176e 100644 --- a/tests/decoder_test.py +++ b/tests/decoder_test.py @@ -550,6 +550,16 @@ def test_oversized_int32_is_bounded(self) -> None: with self.assertRaises(InvalidDatabaseError): Decoder(oversized_int32, pointer_base=0).decode(0) + def test_invalid_extended_type_byte_is_rejected(self) -> None: + # Types below 8 use the type field of the ctrl byte, so extended type + # byte 0 (map) is invalid. Byte 9 would be type 16, which is not + # defined. 0x00: extended type, size 0. + for type_byte, msg in ((0x00, "bad data"), (0x09, "type number")): + with self.subTest(type_byte=type_byte): + buf = bytes([0x00, type_byte]) + with self.assertRaisesRegex(InvalidDatabaseError, msg): + Decoder(buf, pointer_base=0).decode(0) + def test_truncated_data_raises_invalid_database_error(self) -> None: # A ctrl byte past the buffer end, a string header missing its size # bytes, a pointer missing its offset byte, and a map missing its key From cf9b2dbe6ed442a5645512b20a9da01fa77d4f2e Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 6 Nov 2026 19:00:38 +0000 Subject: [PATCH 2/6] Inline the extended type read in _decode _decode called _read_extended for each value with an extended type, such as arrays, booleans and uint64 values. The read and the byte 0 check are now inline, as the comment above _decode asks. The behavior does not change. Co-Authored-By: Claude Opus 5.5 --- maxminddb/decoder.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/maxminddb/decoder.py b/maxminddb/decoder.py index 65a1351..5ac1404 100644 --- a/maxminddb/decoder.py +++ b/maxminddb/decoder.py @@ -349,9 +349,12 @@ def _decode( # noqa: C901, PLR0911, PLR0912 new_offset = offset + 1 ctrl_byte = self._buffer[offset] type_num = ctrl_byte >> 5 - # Extended type + # Extended type. The next byte holds the type number minus 7. if not type_num: - (type_num, new_offset) = self._read_extended(new_offset) + type_num = self._buffer[new_offset] + 7 + new_offset += 1 + if type_num == 7: + raise InvalidDatabaseError(_BAD_DATA) size = ctrl_byte & 0x1F # Sizes under 29 are stored in the ctrl byte, and a pointer's size bits @@ -397,12 +400,6 @@ def _decode( # noqa: C901, PLR0911, PLR0912 msg = f"Unexpected type number ({type_num}) encountered" raise InvalidDatabaseError(msg) - def _read_extended(self, offset: int) -> tuple[int, int]: - next_byte = self._buffer[offset] - if not next_byte: - raise InvalidDatabaseError(_BAD_DATA) - return next_byte + 7, offset + 1 - @staticmethod def _verify_size(expected: int, actual: int) -> None: if expected != actual: From 2896d9b9c15a7dd1404932ab4a2f14aeb863ef4a Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 6 Nov 2026 19:00:55 +0000 Subject: [PATCH 3/6] Check data_end before reading the extended type byte The decoder read the extended type byte with no data_end check. If the last byte of the data section was an extended ctrl byte, it read the next byte as the type. In a Reader, that byte is 0xAB from the metadata marker, so the error was "Unexpected type number (178) encountered". A Decoder with a smaller data_end could return a value. The decoder now raises the bad-data error, as libmaxminddb does. ENG-5762 Co-Authored-By: Claude Opus 5.5 --- maxminddb/decoder.py | 2 ++ tests/decoder_test.py | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/maxminddb/decoder.py b/maxminddb/decoder.py index 5ac1404..94aa0fd 100644 --- a/maxminddb/decoder.py +++ b/maxminddb/decoder.py @@ -351,6 +351,8 @@ def _decode( # noqa: C901, PLR0911, PLR0912 type_num = ctrl_byte >> 5 # Extended type. The next byte holds the type number minus 7. if not type_num: + if new_offset >= self._data_end: + raise InvalidDatabaseError(_BAD_DATA) type_num = self._buffer[new_offset] + 7 new_offset += 1 if type_num == 7: diff --git a/tests/decoder_test.py b/tests/decoder_test.py index 3dd176e..260292c 100644 --- a/tests/decoder_test.py +++ b/tests/decoder_test.py @@ -560,6 +560,16 @@ def test_invalid_extended_type_byte_is_rejected(self) -> None: with self.assertRaisesRegex(InvalidDatabaseError, msg): Decoder(buf, pointer_base=0).decode(0) + def test_extended_type_byte_past_data_end_is_rejected(self) -> None: + # The buffer continues past data_end, as the metadata follows the data + # section in a database. 0x04: array, 0x07: boolean. + for data in (b"\x00\x04", b"\x01\x07"): + with ( + self.subTest(data=data.hex()), + self.assertRaisesRegex(InvalidDatabaseError, "bad data"), + ): + Decoder(data, pointer_base=0, data_end=1).decode(0) + def test_truncated_data_raises_invalid_database_error(self) -> None: # A ctrl byte past the buffer end, a string header missing its size # bytes, a pointer missing its offset byte, and a map missing its key From fa653a593d0b4795243849e07f1441e9c795517f Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 6 Nov 2026 19:01:13 +0000 Subject: [PATCH 4/6] Use one message for invalid extended type bytes Extended type byte 0 gave the generic bad-data message, and bytes 5, 6 and 9 to 255 gave "Unexpected type number (N) encountered". Both now give "Invalid extended type byte (N)", which names the cause. Co-Authored-By: Claude Opus 5.5 --- maxminddb/decoder.py | 7 +++++-- tests/decoder_test.py | 13 ++++++++----- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/maxminddb/decoder.py b/maxminddb/decoder.py index 94aa0fd..7b5e8ff 100644 --- a/maxminddb/decoder.py +++ b/maxminddb/decoder.py @@ -356,7 +356,8 @@ def _decode( # noqa: C901, PLR0911, PLR0912 type_num = self._buffer[new_offset] + 7 new_offset += 1 if type_num == 7: - raise InvalidDatabaseError(_BAD_DATA) + msg = "Invalid extended type byte (0)" + raise InvalidDatabaseError(msg) size = ctrl_byte & 0x1F # Sizes under 29 are stored in the ctrl byte, and a pointer's size bits @@ -399,7 +400,9 @@ def _decode( # noqa: C901, PLR0911, PLR0912 case 15: return self._decode_float(size, new_offset, budget) case _: - msg = f"Unexpected type number ({type_num}) encountered" + # Only an extended type byte can give a type number that is + # not handled above. + msg = f"Invalid extended type byte ({type_num - 7})" raise InvalidDatabaseError(msg) @staticmethod diff --git a/tests/decoder_test.py b/tests/decoder_test.py index 260292c..80028ed 100644 --- a/tests/decoder_test.py +++ b/tests/decoder_test.py @@ -554,11 +554,14 @@ def test_invalid_extended_type_byte_is_rejected(self) -> None: # Types below 8 use the type field of the ctrl byte, so extended type # byte 0 (map) is invalid. Byte 9 would be type 16, which is not # defined. 0x00: extended type, size 0. - for type_byte, msg in ((0x00, "bad data"), (0x09, "type number")): - with self.subTest(type_byte=type_byte): - buf = bytes([0x00, type_byte]) - with self.assertRaisesRegex(InvalidDatabaseError, msg): - Decoder(buf, pointer_base=0).decode(0) + for type_byte in (0x00, 0x09): + with ( + self.subTest(type_byte=type_byte), + self.assertRaisesRegex( + InvalidDatabaseError, rf"extended type byte \({type_byte}\)" + ), + ): + Decoder(bytes([0x00, type_byte]), pointer_base=0).decode(0) def test_extended_type_byte_past_data_end_is_rejected(self) -> None: # The buffer continues past data_end, as the metadata follows the data From 493861d9266135bbd6c222be6fdf2ff86d80277e Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 6 Nov 2026 19:01:19 +0000 Subject: [PATCH 5/6] Test extended type bytes 5, 6 and 255 Bytes 5 and 6 are types 12 and 13, the data cache container and the end marker, which a record cannot hold. Byte 255 is the largest byte. The test now fails if a later change to the type match accepts them. Co-Authored-By: Claude Opus 5.5 --- tests/decoder_test.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/decoder_test.py b/tests/decoder_test.py index 80028ed..c2ac04f 100644 --- a/tests/decoder_test.py +++ b/tests/decoder_test.py @@ -552,9 +552,11 @@ def test_oversized_int32_is_bounded(self) -> None: def test_invalid_extended_type_byte_is_rejected(self) -> None: # Types below 8 use the type field of the ctrl byte, so extended type - # byte 0 (map) is invalid. Byte 9 would be type 16, which is not - # defined. 0x00: extended type, size 0. - for type_byte in (0x00, 0x09): + # byte 0 (map) is invalid. Bytes 5 and 6 are types 12 and 13 (data + # cache container and end marker), which a record cannot hold. Bytes + # 9 to 255 are types 16 to 262, which are not defined. 0x00: extended + # type, size 0. + for type_byte in (0x00, 0x05, 0x06, 0x09, 0xFF): with ( self.subTest(type_byte=type_byte), self.assertRaisesRegex( From 6800b21ae043fbf80cfea02b04ffd2ed748e5499 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 6 Nov 2026 19:02:09 +0000 Subject: [PATCH 6/6] Test extended type byte 0 in each reader mode The decoder test called only Decoder. This test opens a database whose record has extended type byte 0 in each mode. The pure Python reader must raise InvalidDatabaseError. With the bundled libmaxminddb, the C extension must return an empty map. If the bundled libmaxminddb starts to reject the byte, the test fails, and the HISTORY.rst entry needs a change. A system libmaxminddb can already reject it, so the test also accepts InvalidDatabaseError when MAXMINDDB_USE_SYSTEM_LIBMAXMINDDB is set. Co-Authored-By: Claude Opus 5.5 --- tests/reader_test.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/reader_test.py b/tests/reader_test.py index b8c716e..1fb0ce9 100644 --- a/tests/reader_test.py +++ b/tests/reader_test.py @@ -1086,6 +1086,29 @@ def test_non_string_map_key_is_rejected(self) -> None: ): reader.get(self.ipf("1.1.1.1")) + def test_extended_type_byte_0(self) -> None: + # The pure Python reader rejects extended type byte 0. The bundled + # libmaxminddb does not check this byte, so the C extension decodes it + # as an empty map. If the bundled libmaxminddb starts to reject it, + # update HISTORY.rst too. A system libmaxminddb can already reject it. + with tempfile.TemporaryDirectory() as directory: + path = pathlib.Path(directory) / "extended-type-byte-0.mmdb" + path.write_bytes(_database((17, 17), ip_version=4, data=b"\x00\x00")) + with open_database(str(path), self.mode) as reader: + if self.reader_class is maxminddb.reader.Reader: + with self.assertRaisesRegex( + InvalidDatabaseError, r"extended type byte \(0\)" + ): + reader.get(self.ipf("1.1.1.1")) + return + try: + record = reader.get(self.ipf("1.1.1.1")) + except InvalidDatabaseError: + if not os.environ.get("MAXMINDDB_USE_SYSTEM_LIBMAXMINDDB"): + raise + return + self.assertEqual(record, {}) + def test_too_many_constructor_args(self) -> None: with self.assertRaises(TypeError): self.reader_class("README.md", self.mode, 1) # type: ignore[arg-type,call-arg]