The signed comparison in countBefore breaks for the max uint32_t value.
Even though callers now use getOutboundTotal(), the function itself
should be correct for all inputs.
Instead of overloading getOutboundCount() with a magic sentinel value,
add a dedicated getOutboundTotal() method to the PacketManager interface
that returns the total queue size without time filtering.
This eliminates the fragile convention that caused the regression and
makes the two operations — time-filtered count vs total count —
explicitly separate in the API.
PR #1795 changed PacketQueue::countBefore() to use signed 2's complement
arithmetic for millis wraparound safety. However, this broke the
0xFFFFFFFF sentinel pattern used by callers to mean "count all packets
regardless of schedule".
With the signed comparison, countBefore(0xFFFFFFFF) always returns 0,
causing hasPendingWork() to report false and repeaters to sleep with
packets still queued. Stats reporting also shows queue_len as 0.
Add an early-return for the sentinel value before the loop, and document
the sentinel convention on the virtual interface and implementation.
Address review feedback:
- Move the alignment check before the HMAC-SHA256 computation to skip
expensive crypto on obviously malformed packets
- Update MACThenDecrypt header comment to document block-alignment as a
reason for returning zero
Add validation that ciphertext length is a multiple of CIPHER_BLOCK_SIZE
before calling decrypt(). The decrypt() function processes data in 16-byte
AES blocks, rounding up non-aligned input, which can write up to 8 bytes
past the caller's stack buffer. This can be triggered by crafted packets
that pass the 2-byte HMAC check (1-in-65536 chance per attempt), enabling
a persistent denial-of-service attack against repeaters and other nodes.
PacketQueue::countBefore() and PacketQueue::get() use unsigned
comparison (_schedule_table[j] > now) to check if a packet is
scheduled for the future. This breaks when millis() wraps around
after ~49.7 days: packets scheduled just before the wrap appear
to be in the far future and get stuck in the queue.
Use signed subtraction instead, matching the approach already used
by Dispatcher::millisHasNowPassed(). This correctly handles the
wraparound for time differences up to ~24.8 days in either
direction, well beyond the maximum queue delay of 32 seconds.
Daniel Novak