Skip to content

Bound decoder resource use to prevent denial of service (STF-1488) #439

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

Merged
horgh merged 18 commits into from
Sep 9, 2026
Merged

Bound decoder resource use to prevent denial of service (STF-1488) #439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
ec48a5d
Bound decoder work to prevent a pointer fan-out denial of service
oschwald Aug 25, 2026
d35f28e
Bound decoder string and bytes payload to stop amplification
oschwald Aug 27, 2026
faeba4e
Update libmaxminddb to 1.14.0 and test its resource limits
oschwald Aug 27, 2026
24e2682
Report truncated data as InvalidDatabaseError
oschwald Sep 5, 2026
1aea5d0
Skip the size call for ctrl bytes that hold the size
oschwald Sep 5, 2026
d52f54a
Decode pointers with integer arithmetic
oschwald Sep 5, 2026
5621b6c
Tighten the map and array decode loops
oschwald Sep 5, 2026
1e7806d
Dispatch decoder types with match
oschwald Sep 8, 2026
bcd5228
Decode strings inline in the first match case
oschwald Sep 5, 2026
ca1987a
Read search tree nodes with integer arithmetic
oschwald Sep 5, 2026
2d1bea5
Reject pointers that target other pointers
oschwald Sep 9, 2026
da62052
Close reader buffers when initialization fails
oschwald Sep 9, 2026
86d82c6
Validate search tree metadata when opening the database
oschwald Sep 9, 2026
2dc7511
Cover decoder recursion and pointer boundary cases
oschwald Sep 9, 2026
20853e9
Test high nibbles in 28-bit search tree records
oschwald Sep 9, 2026
53622e7
Clarify decoder limits and early tree validation
oschwald Sep 9, 2026
2983246
Bound malformed metadata cleanup tests
oschwald Sep 9, 2026
976b641
Use a dataclass for decoder budgets
oschwald Sep 9, 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
18 changes: 18 additions & 0 deletions HISTORY.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,24 @@
History
-------

3.2.0
Comment thread
oschwald marked this conversation as resolved.
+++++

* Added limits to pure Python record and metadata decoding to prevent denial
of service from crafted databases: 65,536 values, 512 nesting levels, and
2 MiB of string and bytes payload. Exceeding a limit raises
``InvalidDatabaseError``. CPython may reach its recursion limit earlier,
which raises the same error.
* Rejected unsigned integers longer than 16 bytes and signed integers longer
than 4 bytes before reading their payload.
* Updated the vendored libmaxminddb to 1.14.0, which adds the same resource
limits to the C extension.
* Truncated reads that previously raised ``IndexError`` or ``struct.error``
now raise ``InvalidDatabaseError``.

@horgh horgh Sep 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bullet is broader than the code. Truncated payload reads — uint, string, bytes, and int32 with size < 4 — still return silently wrong values rather than raising, because slicing doesn't raise (details in my comment on _decode_uint).

What actually changed is narrower: a truncated ctrl byte, a truncated size code, and a short pointer now raise InvalidDatabaseError. Either scope the bullet to that, or add the four length checks and keep the bullet as written.

🤖 Comment by Claude (Claude Code) on behalf of Will.

@oschwald oschwald Sep 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex, responding on Greg’s behalf.

Kept the existing qualifier: “Truncated reads that previously raised IndexError or struct.error now raise InvalidDatabaseError.” That describes exception conversion without claiming that every truncated payload is rejected. The pre-existing short-slice cases are tracked in ENG-5393. Greg prefers to keep this release note concise.

* The pure Python reader now rejects invalid search tree sizes when opening
a database.
* Improved pure Python lookup performance.

@horgh horgh Sep 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth one more bullet: Reader now raises InvalidDatabaseError at open time for a database whose search tree extends past the end of the file. Previously such a file opened successfully and failed at the first lookup with a different message.

Confirmed against GeoIP2-City-Test-Invalid-Node-Count.mmdb: on main it opens and get() raises "Invalid node in search tree"; on this branch the constructor raises. That's a user-visible move of a failure from lookup to open, and anyone catching around get() but not around open_database() will notice.

🤖 Comment by Claude (Claude Code) on behalf of Will.

@oschwald oschwald Sep 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex, responding on Greg’s behalf.

Added a short changelog bullet in 53622e7 stating that the Python reader rejects invalid search-tree sizes when opening a database.


3.1.1 (2026-03-05)
++++++++++++++++++

Expand Down
5 changes: 5 additions & 0 deletions README.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ The module will return an ``InvalidDatabaseError`` if the database is corrupt
or otherwise invalid. A ``ValueError`` will be thrown if you look up an
invalid IP address or an IPv6 address in an IPv4 database.

The reader also raises ``InvalidDatabaseError`` when one record, or the
database metadata, exceeds its resource limits: 65,536 decoded values, 512
levels of nesting, or 2 MiB of string and bytes data. Real databases stay far

@horgh horgh Sep 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"512 levels of nesting" is stated flatly here but isn't reachable in the default configuration — a stock CPython caller gets InvalidDatabaseError at roughly 490 levels, via RecursionError (measured; details in my comment on _MAX_DEPTH).

HISTORY.rst hedges this correctly ("CPython may reach its recursion limit earlier, which raises the same error"). This is the user-facing document, so it should carry the same hedge — e.g. "at most 512 levels of nesting; CPython's own recursion limit may stop a decode sooner."

🤖 Comment by Claude (Claude Code) on behalf of Will.

@oschwald oschwald Sep 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex, responding on Greg’s behalf.

Added one sentence in 53622e7: “Python’s recursion limit may stop decoding sooner.”

below these limits. Python's recursion limit may stop decoding sooner.

Thread Safety
-------------

Expand Down
2 changes: 1 addition & 1 deletion extension/libmaxminddb
Open in desktop
Submodule libmaxminddb updated 43 files
+14 −1 .github/dependabot.yml
+1 −1 .github/workflows/clang-addresssanitizer.yml
+1 −1 .github/workflows/clang-analyzer.yml
+3 −3 .github/workflows/codeql-analysis.yml
+123 −0 .github/workflows/dependabot-failure-watcher.yml
+32 −0 .github/workflows/links.yml
+54 −0 .github/workflows/pages.yml
+2 −2 .github/workflows/precious.yml
+2 −2 .github/workflows/test.yml
+2 −2 .github/workflows/zizmor.yml
+3 −1 .gitignore
+0 −3 .gitmodules
+1 −1 CMakeLists.txt
+36 −0 Changes.md
+3 −2 README.dev.md
+1 −1 README.fuzzing.md
+3 −3 README.md
+1 −1 configure.ac
+0 −43 dev-bin/release.sh
+44 −3 doc/libmaxminddb.md
+189 −0 docs/assets/css/main.css
+18 −0 docs/hugo.toml
+24 −0 docs/layouts/404.html
+4 −0 docs/layouts/_default/_markup/render-heading.html
+27 −0 docs/layouts/_default/default.html
+2 −0 include/maxminddb.h
+60 −0 lychee.toml
+135 −19 mise.lock
+14 −0 mise.toml
+28 −12 src/data-pool.c
+11 −7 src/data-pool.h
+179 −33 src/maxminddb.c
+3 −0 t/CMakeLists.txt
+7 −4 t/Makefile.am
+55 −0 t/bad_search_tree_t.c
+56 −10 t/data-pool-t.c
+266 −0 t/decoder_limits_t.pl
+41 −15 t/fuzz_mmdb.c
+51 −0 t/invalid_sockaddr_t.c
+5 −5 t/max_depth_t.c
+1 −1 t/maxmind-db
+23 −0 t/metadata_marker_t.c
+238 −0 t/pointer_dos_t.c
Loading
Loading