bucket-A4: cap Packet read-constructor prefix_length (SAFE-ADDITIVE, Bug-9 hardening)

[frstrtr]

Summary

Adds a bounds guard to Packet(size_t prefix_length): caps prefix_length at 16 and throws std::ios_base::failure if exceeded, instead of letting prefix.resize() throw std::length_error (vector::_M_default_append) and abort the process. Additive hardening — no change to valid-traffic behavior (audit classification: SAFE-ADDITIVE).

1 file, +11:

• modified — src/core/packet.hpp

Cherry-picked from dash-spv-embedded HEAD 8249dc28.

Context

Part 4 of the bucket-A sub-PRs for the multi-coin landing strategy. Follows #46 (A1), #48 (A2), #49 (A3). Independent of A1–A3 (different file, no shared symbols).

Why this is safe for LTC+DOGE

• prefix_length flows from m_node->get_prefix().size() in Socket::init(). All known protocols use a 4-byte magic prefix, so the cap at 16 is never reached in normal operation — the resize(prefix_length) path is unchanged for every valid connection.
• The guard only fires in the Bug-3 UAF class (rapid disconnect-reconnect destroys m_node mid-handshake → the vector's size field reads garbage → unguarded resize() aborts). It converts that crash into a clean per-connection failure (std::ios_base::failure, already the connection-teardown exception type).
• This is the exact line that has run in the Dash mainnet soak for weeks (it's part of the Bug-9 hardening set).

CI expectation

• Legacy linux: + cross-platform + CodeQL: green.
• coin-matrix entries: skip fast on this branch.

Reviewer

Agent-side reviewer: ltc-doge-production-steward (watches the branch). Integrator (cc) routes human review.

Test plan

• Legacy linux: CI green (full test suite — exercises the Packet read path on valid LTC/DOGE traffic)
• CodeQL + cross-platform green
• No file other than src/core/packet.hpp touched (directory-contract spot-check)

[frstrtr]

Review — bucket-A4 (#50): Packet prefix_length cap, Bug-9 hardening

Verdict: APPROVE. SAFE-ADDITIVE label verified. All bound-relevant points check out; one non-blocking gap (no unit test for the cap). Bound-by-bound notes below.

1. Cap value / correctness

constexpr size_t MAX_PREFIX_LENGTH = 16, hardcoded — not derived from a protocol constant. All known protocols use a 4-byte magic prefix (get_prefix() in impl/ltc/coin/p2p_node.hpp:252, pool/node.hpp:86), so 16 is 4x headroom: tight enough to stop the OOM/abort, far below any valid value so legitimate traffic is never rejected. Minor/non-blocking: the constant is not tied to the actual max known prefix, so a future >16-byte protocol prefix would be silently rejected at handshake. A comment referencing the documented max prefix (or a static check) would future-proof it.

2. Off-by-one / overflow

Operator is > (16 passes, 17 throws). Boundary choice is immaterial since valid = 4. prefix_length is size_t and the caller passes m_node->get_prefix().size() (size_t) — no signed/unsigned mismatch, no truncation across the call, no overflow in a size_t > size_t compare. Clean.

3. Error path

Throws std::ios_base::failure, caught at Socket::read() catch (const std::exception&) (socket.cpp:99-103) -> LOG_WARNING + abort_connection(). ios_base::failure is system_error -> runtime_error -> exception, so it is caught; it is also the established parse/teardown exception type across this codebase (pack.hpp, transaction.hpp, pack_test.cpp). Cap-out (synchronous ctor throw, before any read) is cleanly distinct from EOF/short-read (async callback if (ec) in read_prefix). Clean separation.

4. Fuzz / DoS — KEY CLARIFICATION

prefix_length is NOT peer-controlled. It flows solely from m_node->get_prefix().size() (the LOCAL node's configured magic prefix), never from wire data. A malformed peer cannot influence it, so the "repeated max-cap packet" DoS vector does not apply here. The actual trigger is the Bug-3-family UAF (rapid disconnect-reconnect frees m_node mid-handshake -> garbage size read). That race is now ALSO guarded at the call site by acquire_node() (socket.cpp:84-95). So this ctor cap is genuine belt-and-braces defense-in-depth, exactly as both code comments state. Each trip = one clean abort_connection(), no unbounded resource growth, no in-process retry storm.

5. Include hygiene (minor)

packet.hpp carries no #includes and relies on transitive includes (via socket.hpp) for std::ios_base::failure / std::vector. CI is green on Linux x86_64, Windows, macOS, so it resolves everywhere, and this matches the file's pre-existing style — not a regression. A direct include would be cleaner but is non-blocking.

6. Test coverage — the one real gap (non-blocking)

No unit test exercises the cap. No test file references Packet/prefix, and the PR touches only packet.hpp. The Linux suite exercises only the valid 4-byte resize path, never the >16 throw. For a security-relevant fix I would add a small GTest: at-cap (16 -> ok), just-over (17 -> throws ios_base::failure), far-over (SIZE_MAX -> throws). The ctor is header-only and trivially testable; pack_test.cpp:35 already shows the ASSERT_THROW(..., std::ios_base::failure) pattern. Recommend adding before merge, but not a blocker on the fix itself.

CI

All green: Linux x86_64 ctest, Windows, macOS arm64, CodeQL, plus btc/ltc/doge/dash smokes. Embedded-smoke skip-fast as expected for this branch.

Approvable as-is. Recommend the cap-boundary unit test (point 6) as a fast follow.

— ltc-doge-production-steward