fix(unmarshaller): cap message size at 128 MiB per d-bus spec

[bdraco]

What

Enforce the D-Bus spec 128 MiB total-message-size cap in the unmarshaller. _read_header now validates body_len and header_len (and their sum) immediately after reading them, before _msg_len is computed and before _read_to_pos can size an allocation or recvmsg from those untrusted values.

Why

_read_header previously consumed both length fields as raw uint32s from the wire, then handed HEADER_SIGNATURE_SIZE + _msg_len to _read_to_pos. A single 28-byte forged header claiming body_len = 0xFFFFFFFF forced the victim to attempt to buffer ~4 GiB before any validation — a remote DoS against any peer that subscribes to messages on a shared system bus.

Captured as #641; the TODO at message.py:324 noted the missing cap but only on the outbound (marshal) side.

How

• Added MAX_MESSAGE_SIZE = 134_217_728 (Python-importable) and _MAX_MESSAGE_SIZE (cdef'd unsigned int) so the bounds check compiles to a native C compare on the hot path.
• In _read_header, raise InvalidMessageError if either field — or their sum — exceeds the cap. The check runs before _msg_len is computed, so no allocation or socket read is ever attempted at the attacker-chosen size.

Tests

Four new tests in tests/test_marshaller.py build forged 16-byte headers (no body bytes follow) and assert InvalidMessageError is raised — the absence of a body proves validation runs before any body read:

• test_unmarshall_rejects_oversized_body_len
• test_unmarshall_rejects_oversized_header_len
• test_unmarshall_rejects_oversized_combined_size
• test_unmarshall_rejects_max_uint32_body_len_big_endian — the original body_len=0xFFFFFFFF payload from the issue

closes #641

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.11%. Comparing base (5d181d5) to head (f683b17).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #653      +/-   ##
==========================================
- Coverage   89.16%   89.11%   -0.05%     
==========================================
  Files          29       29              
  Lines        3488     3492       +4     
  Branches      602      603       +1     
==========================================
+ Hits         3110     3112       +2     
- Misses        228      229       +1     
- Partials      150      151       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[codspeed-hq]

Merging this PR will not alter performance

✅ 6 untouched benchmarks

---

_{Comparing fix/unmarshaller-message-size-cap (f683b17) with main (ed75962)}

[bdraco]

@bluetoothbot review

[Copilot]

Pull request overview

This PR hardens dbus_fast’s unmarshaller against attacker-controlled length fields by enforcing the D-Bus 128 MiB message-size cap early in header parsing, preventing oversized buffering/reads and mitigating a remote memory-exhaustion DoS (issue #641).

Changes:

• Add a shared 128 MiB MAX_MESSAGE_SIZE constant (plus a Cython-typed _MAX_MESSAGE_SIZE) in the unmarshaller.
• Validate body_len, header_len, and their combination in _read_header() and raise InvalidMessageError when exceeding the cap.
• Add tests that forge minimal headers and assert oversized lengths are rejected.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/dbus_fast/_private/unmarshaller.py	Introduces the max-size constants and performs early header/body length validation.
src/dbus_fast/_private/unmarshaller.pxd	Declares _MAX_MESSAGE_SIZE for Cython compilation.
tests/test_marshaller.py	Adds tests that forge headers to confirm oversized length fields are rejected.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[bluetoothbot]

PR Review — fix(unmarshaller): cap message size at 128 MiB per d-bus spec

Targeted, well-documented DoS fix. The dual-name MAX_MESSAGE_SIZE / _MAX_MESSAGE_SIZE pattern matches the cython-gotchas guidance in CLAUDE.md, the .pxd declaration is in place, and the validation is correctly positioned before _msg_len is computed and before any allocation or recvmsg sized by attacker-controlled fields. Tests forge 16-byte headers with no trailing body bytes, which is a good way to prove the check fires pre-allocation. The 23-byte slack (HEADER_SIGNATURE_SIZE + up-to-7 alignment padding) is acknowledged in the comment block and is a reasonable trade-off — the DoS vector is gigabytes-from-a-uint32, not 23 bytes over spec. One optional suggestion: a positive boundary test at exactly MAX_MESSAGE_SIZE would pin the inequality direction. Merge-ready.

---

🟢 Suggestions

1. Consider adding a boundary-pass test at the exact cap (`tests/test_marshaller.py`, L1017)

The four new tests all assert rejection above the cap. A complementary positive test — e.g. body_len = MAX_MESSAGE_SIZE, header_len = 0 — would pin the boundary in the other direction and document that exactly-128-MiB messages are still accepted (a benign-but-legal-by-spec case). It also guards against a future tightening that off-by-ones the comparison to >=. Not blocking given bdraco's earlier note that tightening the math to the byte is overkill; but a single equality-edge test is cheap insurance.

def test_unmarshall_accepts_body_len_at_cap() -> None:
    forged = _forged_header(body_len=MAX_MESSAGE_SIZE, header_len=0)
    # Will fail later on missing body bytes, not on the cap check
    with pytest.raises((IndexError, MarshallerStreamEndError)):
        Unmarshaller(io.BytesIO(forged)).unmarshall()

2. Short-circuit ordering covers overflow but worth a note (`src/dbus_fast/_private/unmarshaller.py`, L811-820)

Minor: the three-way OR is correct because the sum > cap branch is only ever evaluated after both individual checks pass, so the sum is bounded by 2 * _MAX_MESSAGE_SIZE = 256 MiB and can't wrap unsigned int in the Cython compile. If the individual checks were ever removed or reordered, the sum check would become vulnerable to wraparound (e.g. 0xFFFFFFFF + 1 == 0). Not asking for a code change — the current ordering is fine — but a one-line comment on the dependency would help a future reader avoid re-ordering these for stylistic reasons.

---

Checklist

• Input validation at system boundaries
• No unsafe deserialization or path traversal
• Cython .pxd updated in same commit as cdef'd module constant
• Dual-name pattern for Python-importable cdef constant
• No unsigned int-via-cdef int sign-flip risk on new check
• Tests cover the attack vector (oversized body_len, header_len, sum, max-uint32)
• Boundary-equality test (value == cap) — suggestion ＃1
• Error message doesn't leak internal state to end users
• Conventional Commits subject, lowercase, correct type (fix: for user-visible bugfix)
• No Co-Authored-By trailers for LLM tooling

---

Summary

Targeted, well-documented DoS fix. The dual-name MAX_MESSAGE_SIZE / _MAX_MESSAGE_SIZE pattern matches the cython-gotchas guidance in CLAUDE.md, the .pxd declaration is in place, and the validation is correctly positioned before _msg_len is computed and before any allocation or recvmsg sized by attacker-controlled fields. Tests forge 16-byte headers with no trailing body bytes, which is a good way to prove the check fires pre-allocation. The 23-byte slack (HEADER_SIGNATURE_SIZE + up-to-7 alignment padding) is acknowledged in the comment block and is a reasonable trade-off — the DoS vector is gigabytes-from-a-uint32, not 23 bytes over spec. One optional suggestion: a positive boundary test at exactly MAX_MESSAGE_SIZE would pin the inequality direction. Merge-ready.

---

Automated review by Kōanc703d8f
f683b17

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.