Skip to content

Reject truncated values and non-string map keys #463

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
oschwald wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Reject truncated values and non-string map keys #463

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
aaf2e31
Reject values past the end of the data section
oschwald Oct 20, 2026
4b73cf2
Reject non-string map keys in the pure Python reader
oschwald Oct 20, 2026
3757088
Test non-string map keys in every reader mode
oschwald Oct 20, 2026
617b9ea
Measure the database buffer with len()
oschwald Oct 20, 2026
e5b6009
Decode map keys without the generic dispatch
oschwald Oct 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions HISTORY.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,12 @@ History
C extension. Before, the iterator walked the new database with node
numbers from the old one. A failed ``__init__`` keeps the old database.
After ``close()``, an iterator raises ``ValueError`` in every mode.
* For a record map with a key that cannot be hashed, such as a list, the
pure Python reader raises ``InvalidDatabaseError`` instead of ``TypeError``,
as the C extension does.
* The pure Python reader now raises ``InvalidDatabaseError`` when a value
extends past the end of the data section. Previously, it could return a
shorter value or decode bytes from the metadata section.
* 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``.
* C extension:

* Fixed segmentation faults from invalid use of ``Metadata``, ``Reader`` and
Expand Down
82 changes: 71 additions & 11 deletions maxminddb/decoder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@
"The MaxMind DB file's data section contains bad data "
"(unknown data type or corrupt data)"
)
# The same message as the C extension.
_NON_STRING_KEY = "Invalid map key: the key is not a string."


@dataclass
Expand All @@ -68,18 +70,24 @@ def __init__(
database_buffer: FileBuffer | mmap.mmap | bytes | bytearray,
pointer_base: int = 0,
pointer_test: bool = False, # noqa: FBT001, FBT002
data_end: int | None = None,
) -> None:
"""Create a Decoder for a MaxMind DB.

Arguments:
database_buffer: an mmap'd MaxMind DB file.
pointer_base: the base number to use when decoding a pointer
pointer_test: used for internal unit testing of pointer code
data_end: the offset where the decoded section ends. No value may
extend past it. The default is the end of the buffer.

"""
self._pointer_test = pointer_test
self._buffer = database_buffer
self._pointer_base = pointer_base
if data_end is None:
data_end = len(database_buffer)
self._data_end = data_end

def _decode_array(
self,
Expand Down Expand Up @@ -124,6 +132,8 @@ def _decode_bytes(
raise InvalidDatabaseError(_TOO_LARGE)
budget.payload_left = remaining
new_offset = offset + size
if new_offset > self._data_end:
raise InvalidDatabaseError(_BAD_DATA)
return self._buffer[offset:new_offset], new_offset

def _decode_double(
Expand All @@ -134,6 +144,8 @@ def _decode_double(
) -> tuple[float, int]:
self._verify_size(size, 8)
new_offset = offset + size
if new_offset > self._data_end:
raise InvalidDatabaseError(_BAD_DATA)
packed_bytes = self._buffer[offset:new_offset]
(value,) = struct.unpack(b"!d", packed_bytes)
return value, new_offset
Expand All @@ -146,6 +158,8 @@ def _decode_float(
) -> tuple[float, int]:
self._verify_size(size, 4)
new_offset = offset + size
if new_offset > self._data_end:
raise InvalidDatabaseError(_BAD_DATA)
packed_bytes = self._buffer[offset:new_offset]
(value,) = struct.unpack(b"!f", packed_bytes)
return value, new_offset
Expand All @@ -161,6 +175,8 @@ def _decode_int32(
if size == 0:
return 0, offset
new_offset = offset + size
if new_offset > self._data_end:
raise InvalidDatabaseError(_BAD_DATA)
packed_bytes = self._buffer[offset:new_offset]

if size != 4:
Expand All @@ -185,17 +201,54 @@ def _decode_map(
budget.depth = depth
container: dict[str, Record] = {}
decode = self._decode
decode_key = self._decode_key
for _ in range(size):
(key, offset) = decode(offset, budget, False) # noqa: FBT003
(key, offset) = decode_key(offset, budget)
(value, offset) = decode(offset, budget, False) # noqa: FBT003
try:
container[key] = value # type: ignore[index]
except TypeError as ex:
# The key cannot be hashed, such as a list.
raise InvalidDatabaseError(_BAD_DATA) from ex
container[key] = value
budget.depth -= 1
return container, offset

def _decode_key(self, offset: int, budget: _DecodeBudget) -> tuple[str, int]:
# A key is a string or a pointer to one. Real databases share most keys
# through pointers, so decode both here without the generic dispatch.
# A string cannot nest, so a pointer key does not count toward the
# depth.
buffer = self._buffer
data_end = self._data_end
ctrl_byte = buffer[offset]
new_offset = offset + 1
# The map continues after the pointer, not after its target.
pointer_end = 0
if ctrl_byte >> 5 == 1:
size = ctrl_byte & 0x1F
pointer_size = (size >> 3) + 1
pointer_end = new_offset + pointer_size
if pointer_end > data_end:
raise InvalidDatabaseError(_BAD_DATA)
pointer = int.from_bytes(buffer[new_offset:pointer_end], "big")
if pointer_size < 4:
pointer |= (size & 0x7) << (pointer_size << 3)
pointer += _POINTER_VALUE_OFFSETS[pointer_size]
pointer += self._pointer_base
if pointer >= data_end:
raise InvalidDatabaseError(_BAD_DATA)
ctrl_byte = buffer[pointer]
new_offset = pointer + 1
if ctrl_byte >> 5 != 2:
raise InvalidDatabaseError(_NON_STRING_KEY)
size = ctrl_byte & 0x1F
if size >= 29:
(size, new_offset) = self._size_from_ctrl_byte(size, new_offset)
remaining = budget.payload_left - size
if remaining < 0:
raise InvalidDatabaseError(_TOO_LARGE)
budget.payload_left = remaining
end = new_offset + size
if end > data_end:
raise InvalidDatabaseError(_BAD_DATA)
return buffer[new_offset:end].decode("utf-8"), pointer_end or end

def _decode_pointer(
self,
size: int,
Expand All @@ -204,10 +257,9 @@ def _decode_pointer(
) -> tuple[Record, int]:
pointer_size = (size >> 3) + 1
new_offset = offset + pointer_size
pointer_bytes = self._buffer[offset:new_offset]
if len(pointer_bytes) != pointer_size:
if new_offset > self._data_end:
raise InvalidDatabaseError(_BAD_DATA)
pointer = int.from_bytes(pointer_bytes, "big")
pointer = int.from_bytes(self._buffer[offset:new_offset], "big")
if pointer_size < 4:
# The low three bits of the ctrl byte are the high bits of the
# pointer, and sizes 2 and 3 add a fixed offset.
Expand All @@ -218,6 +270,11 @@ def _decode_pointer(
if self._pointer_test:
return pointer, new_offset

# The payload checks do not cover a target with no payload, such as
# an empty map.
if pointer >= self._data_end:
raise InvalidDatabaseError(_BAD_DATA)

# The value at the pointer's position was charged by its containing
# array or map, so the target costs nothing more. Only the depth changes.
depth = budget.depth + 1
Expand All @@ -239,8 +296,9 @@ def _decode_uint(
if size > _MAX_UINT_BYTES:
raise InvalidDatabaseError(_BAD_DATA)
new_offset = offset + size
uint_bytes = self._buffer[offset:new_offset]
return int.from_bytes(uint_bytes, "big"), new_offset
if new_offset > self._data_end:
raise InvalidDatabaseError(_BAD_DATA)
return int.from_bytes(self._buffer[offset:new_offset], "big"), new_offset

def decode(self, offset: int) -> tuple[Record, int]:
"""Decode a section of the data section starting at offset.
Expand Down Expand Up @@ -297,6 +355,8 @@ def _decode( # noqa: C901, PLR0911, PLR0912
raise InvalidDatabaseError(_TOO_LARGE)
budget.payload_left = remaining
end = new_offset + size
if end > self._data_end:
raise InvalidDatabaseError(_BAD_DATA)
return self._buffer[new_offset:end].decode("utf-8"), end
case 1:
if pointer_target:
Expand Down
16 changes: 14 additions & 2 deletions maxminddb/file.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import os
from typing import TYPE_CHECKING, overload

from maxminddb.errors import InvalidDatabaseError

try:
from multiprocessing import Lock
except ImportError:
Expand Down Expand Up @@ -33,7 +35,17 @@ def __getitem__(self, index: slice) -> bytes: ...
def __getitem__(self, index: slice | int) -> bytes | int:
"""Get item by index."""
if isinstance(index, slice):
return self._read(index.stop - index.start, index.start)
size = index.stop - index.start
data = self._read(size, index.start)
# The readers check offsets against the size at open. A short read
# means that the file is corrupt or that it shrank after open.
if len(data) != size:
msg = (
"The read went past the end of the database file. The file "
"is corrupt, or it changed after the reader opened it."
)
raise InvalidDatabaseError(msg)
return data
if isinstance(index, int):
return self._read(1, index)[0]
msg = "Invalid argument type."
Expand All @@ -46,7 +58,7 @@ def rfind(self, needle: bytes, start: int) -> int:
return pos
return start + pos

def size(self) -> int:
def __len__(self) -> int:
"""Size of file."""
return self._size

Expand Down
33 changes: 14 additions & 19 deletions maxminddb/reader.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ class Reader:
_METADATA_START_MARKER = b"\xab\xcd\xefMaxMind.com"

_buffer: bytes | bytearray | FileBuffer | "mmap.mmap" # noqa: UP037
_buffer_size: int
# The offset of the metadata marker, where the data section ends.
_data_end: int
# No database is open until __init__ succeeds.
closed: bool = True
_decoder: Decoder
Expand Down Expand Up @@ -125,7 +126,7 @@ def _load(
filename = self._load_buffer(database, mode)
metadata_start = self._buffer.rfind(
self._METADATA_START_MARKER,
max(0, self._buffer_size - 128 * 1024),
max(0, len(self._buffer) - 128 * 1024),
)

if metadata_start == -1:
Expand All @@ -137,6 +138,7 @@ def _load(
msg,
)

self._data_end = metadata_start
metadata_start += len(self._METADATA_START_MARKER)
metadata_decoder = Decoder(self._buffer, metadata_start)
# For a repeated key, the decoder keeps the last value, but
Expand Down Expand Up @@ -167,14 +169,16 @@ def _load(

# Traversal reads nodes below node_count. Once the tree fits, those
# reads need no length checks of their own.
if self._data_start > self._buffer_size:
if self._data_start > self._data_end:
msg = (
f"Error opening database file ({filename}). The search tree "
"extends past the end of the file."
"extends into the metadata section."
)
raise InvalidDatabaseError(msg) # noqa: TRY301

self._decoder = Decoder(self._buffer, self._data_start)
self._decoder = Decoder(
self._buffer, self._data_start, data_end=self._data_end
)
self.closed = False

ipv4_start = 0
Expand Down Expand Up @@ -362,9 +366,9 @@ def _read_node(self, node_number: int, index: int) -> int:
def _resolve_data_pointer(self, pointer: int) -> Record:
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.
if resolved < self._data_start or resolved >= self._buffer_size:
# A record must point into the data section. libmaxminddb also rejects
# a pointer into the separator between the tree and the data section.
if resolved < self._data_start or resolved >= self._data_end:
raise InvalidDatabaseError(_CORRUPT_TREE)

(data, _) = self._decoder.decode(resolved)
Expand Down Expand Up @@ -414,15 +418,11 @@ def _load_path(self, database: StrOrBytesPath | int, mode: int) -> object:
if (mode == MODE_AUTO and mmap) or mode == MODE_MMAP:
with open(database, "rb") as db_file:
self._buffer = mmap.mmap(db_file.fileno(), 0, access=mmap.ACCESS_READ)
self._buffer_size = self._buffer.size()
elif mode in (MODE_AUTO, MODE_FILE):
self._buffer = FileBuffer(database)
self._buffer_size = self._buffer.size()
elif mode == MODE_MEMORY:
with open(database, "rb") as db_file:
buf = db_file.read()
self._buffer = buf
self._buffer_size = len(buf)
self._buffer = db_file.read()
else:
msg = f"Unsupported open mode ({mode}) for a path or a file descriptor."
raise ValueError(msg)
Expand All @@ -444,7 +444,6 @@ def _load_file_object(self, database: object, read: Callable[[], object]) -> obj
msg += " Open it in binary mode."
raise TypeError(msg)
self._buffer = buf
self._buffer_size = len(buf)
# io buffers are not guaranteed to have a name attribute
if hasattr(database, "name"):
return database.name
Expand Down Expand Up @@ -501,10 +500,6 @@ def _metadata_fields(metadata: RecordDict, filename: object) -> dict[str, Any]:
A new minor version of the format can add keys. This ignores them.
"""
prefix = f"Error reading metadata in database file ({filename})."
# The C extension also rejects a key that is not a string.
if not all(type(k) is str for k in metadata):
msg = f"{prefix} A metadata key is not a string."
raise InvalidDatabaseError(msg)
fields: dict[str, Any] = {}
for key, value_type in _METADATA_TYPES.items():
value = metadata.get(key)
Expand All @@ -513,7 +508,7 @@ def _metadata_fields(metadata: RecordDict, filename: object) -> dict[str, Any]:
if valid and isinstance(value, list):
valid = all(type(v) is str for v in value)
elif valid and isinstance(value, dict):
valid = all(type(k) is str and type(v) is str for k, v in value.items())
valid = all(type(v) is str for v in value.values())
if not valid:
msg = f"{prefix} The {key} value is missing or has the wrong type."
raise InvalidDatabaseError(msg)
Expand Down
Loading
Loading