fix: bound the inbound QoS 2 receive window (GHSA-5jvp-3p2v-5qgf)

[robertsLando]

Fixes GHSA-5jvp-3p2v-5qgf. Reported by Team Atlanta.

Builds on the GHSA-p8r9 fix, already on main as c7acaa1 — the dependency is real, not packaging: see
"Why it needs p8r9". Also needs aedes-persistence 11.0.0.

The bug

Inbound QoS 2 packet bodies were retained in the per-client incoming persistence map with no bound on how many
a single connection could accumulate — queueLimit only guards the pre-CONNACK parser queue. Unbounded
attacker-controlled retention, so memory growth until the broker degrades.

Trigger and full analysis are in the advisory; they stay there until it publishes.

Two things the first attempt got wrong

The original fix used a per-client integer counter, checked before await incomingStorePacket and incremented
after. Deep review found it did not work, and I reproduced both failures:

1. The cap was bypassable by the exact flood it targets. enqueue in lib/client.js dispatches every packet
of a socket read chunk concurrently, so a whole chunk passed the check before any increment landed — the cap was
bypassable by the exact flood it targets, with no clientError raised.

2. It broke legitimate clients. Two concurrent PUBLISHes with the same messageId — an ordinary QoS 2 DUP
retransmission — both missed the store, both stored, both incremented; the single PUBREL decremented once. The
counter drifted upward until a well-behaved retransmitting client sat at the cap and got dropped. An availability
regression caused by the fix.

The fix

The slot is reserved synchronously, before the store, and keyed by messageId. One change closes both:
reserving at the check beats the concurrent chunk, and keying by messageId means a DUP re-reserves its own slot
instead of consuming a second one.

On this branch the same regression test reports maxInflightInbound exceeded and the cap holds.

Also fixed:

• pubrel releases the slot only when incomingDelPacket actually resolves. Freeing it after a failed delete
  would let the store grow past the cap. The rejection is now handled rather than riding out of .finally()
  uncaught, where a duplicate PUBREL losing the race killed the process.
• maxInflightInbound is clamped. -1, NaN, 1.5 or a non-numeric config previously took the limit > 0
  false branch and silently disabled the security control. They now fall back to the default; only an explicit 0
  opts out.
• The error names the option and the limit, matching queueLimit's Client queue limit reached convention,
  instead of borrowing MQTT 5 "Receive Maximum" vocabulary for a v3-only feature.

Why it needs p8r9

The counter is per-connection; the store it bounds is per-clientId. Without p8r9 nothing ever cleared the incoming
store, so a client could reconnect into a fresh budget while its previous packets were still retained — 655
reconnects still reaches the 65535 ceiling. p8r9 discards a clean session's store when the connection ends, which
gives the reservation and the data it represents the same lifetime. On its own this PR bounds a connection, not a
session.

Default is 1000, not 100

MQTT 3.1.1 has no Receive Maximum to advertise the window, so a pipelining publisher cannot know to throttle.
Benchmarked: 100 is inside the range mqtt.js reaches on a burst (60 publishes/chunk × 3 chunks was dropped), which
would have disconnected legitimate publishers on a patch release. 1000 still bounds the map far below the 16-bit
messageId ceiling. 0 restores pre-fix behaviour and is documented as doing exactly that.

Known limitation, documented

This caps packets, not bytes. aedes sets no maximum packet size and mqtt-packet permits a 256 MB remaining
length, so the byte bound is maxInflightInbound × max frame size. A byte budget or a frame limit is a separate
change; docs/Aedes.md says so rather than implying a memory bound the code doesn't deliver.

Separately: growth across distinct client ids is a different axis that no per-client cap closes. Worth its own
advisory.

Tests

Seven cases in test/qos2.js, written against what the review actually found rather than the happy path:

• the cap holds against a 40-packet pipelined flood in one chunk (the bypass)
• a DUP retransmission does not consume a second slot (the availability regression)
• completing PUBREL frees the slot — a well-behaved client is never throttled
• a duplicate PUBREL does not crash and does not free a slot twice
• maxInflightInbound: 0 disables the cap
• a bad maxInflightInbound falls back to the default instead of disabling the cap
• the original boundary case

Full suite 327 pass / 0 fail, lint and tsd clean.

---

Cherry-picked from the advisory's private fork (b490a43) onto main, so the merge carries a subject the
changelog can render. Verified here against the released aedes-persistence@11.0.0: 372 pass / 0 fail
(6 platform skips), lint and tsd clean.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.90%. Comparing base (c7acaa1) to head (dfac7d2).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1154   +/-   ##
=======================================
  Coverage   99.90%   99.90%           
=======================================
  Files          15       15           
  Lines        2077     2127   +50     
=======================================
+ Hits         2075     2125   +50     
  Misses          2        2           

Flag	Coverage Δ
unittests	99.90% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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.