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..7b5e8ff 100644 --- a/maxminddb/decoder.py +++ b/maxminddb/decoder.py @@ -349,9 +349,15 @@ 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) + if new_offset >= self._data_end: + raise InvalidDatabaseError(_BAD_DATA) + type_num = self._buffer[new_offset] + 7 + new_offset += 1 + if type_num == 7: + 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 @@ -394,22 +400,11 @@ 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) - 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 - @staticmethod def _verify_size(expected: int, actual: int) -> None: if expected != actual: diff --git a/tests/decoder_test.py b/tests/decoder_test.py index c2820a3..c2ac04f 100644 --- a/tests/decoder_test.py +++ b/tests/decoder_test.py @@ -550,6 +550,31 @@ 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. 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( + 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 + # 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 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]