internal: reject inbound packets that are not block aligned

[yosuke-wolfssl]

Problem

DoReceive() accepted an inbound binary packet on a maximum-length check alone. Nothing verified the RFC 4253 §6 rule that packet_length + padding_length + payload + padding be a multiple of the cipher block size, or 8, whichever is larger. RFC 5647 §7.2 imposes the same rule on AES-GCM, excluding the length field, which it treats as AAD.

What coverage existed was incidental. AES-CBC/CTR were aligned only as a side effect of Decrypt()'s sz % AES_BLOCK_SIZE, and only when the packet spanned more than one block. AES-GCM had no check at all. Neither did the null cipher — which every connection runs for the whole pre-NEWKEYS handshake, in every build, with no MAC to backstop it. So KEXINIT, KEXDH, and NEWKEYS were all accepted at arbitrary lengths.

Closes f-8834.

Fix (src/internal.c)

One check in DoReceive()'s PROCESS_PACKET_LENGTH case, placed before any decrypt or dispatch so it covers every cipher path:

Transport	Aligned quantity	Block size
cleartext / null cipher	UINT32_SZ + packet_length	8
AES-CBC, AES-CTR	UINT32_SZ + packet_length	16
AES-GCM	packet_length	16

peerBlockSz is floored at MIN_BLOCK_SZ. A misaligned packet sets ssh->error to WS_BUFFER_E and returns WS_FATAL_ERROR — the code DoPacket() already returns for the sibling MIN_PAD_LENGTH violation. The rule mirrors what BundlePacket() has always emitted on the send side, so no conformant peer is affected.

Tests

test_DoReceive_RejectsMisalignedPacket, test_DoReceive_RejectsMisalignedCtr, and test_DoReceive_RejectsMisalignedAead cover the cleartext, AES-CTR, and AES-GCM paths. Each packet is valid in every other respect, so alignment is the only thing that can reject it. The AES-CTR case is what pins the UINT32_SZ + term: its packet_length of 16 is aligned as a body on its own and misaligned only once the length field is counted, a shape neither of the other two exercises.

Fixture churn: BuildMacTestPacketPrefix() (unit.c) aligned the packet body to 8 and left the total at 12, i.e. it was emitting packets RFC 4253 forbids. It now takes padLen from the caller and pads to a block-aligned total; test_DoReceive_VerifyMacFailure, test_DoReceive_AeadTagFailure, and test_DoReceive_RejectsShortPadding follow, and the AEAD test uses the real GCM peerBlockSz of 16 rather than 4. The matching BuildPacket() change in regress.c is a drive-by tidy rather than a consequence of the fix — those packets only ever reach ParseMsgId(), never DoReceive().

Verification

• gcc-13 build sweep, 6 configs: all clean, zero warnings; lint clean.
• make check on --enable-all: 12 tests, 1 skip, 0 real failures. scripts/sftp.test and scripts/scp.test fail only under the parallel run and pass serially — a known flake, unrelated to this change.
• -DWOLFSSH_NO_AES_CTR build: the new test compiles out, no warnings, no link error, remaining tests pass.
• Negative controls: reverting the check fails all three new tests; dropping just the UINT32_SZ + term fails only the AES-CTR test, which the other two do not catch.

[Copilot]

Pull request overview

This PR hardens inbound SSH packet parsing by enforcing RFC-required packet block alignment in DoReceive(), ensuring malformed packets are rejected consistently across cleartext/null, CBC/CTR, and AEAD (AES-GCM) transport paths.

Changes:

• Add a block-alignment check in src/internal.c during PROCESS_PACKET_LENGTH, before decrypt/dispatch.
• Fix test fixtures that previously emitted RFC-invalid packet sizes and add targeted negative tests for misalignment (cleartext + AES-GCM).
• Adjust existing AEAD/HMAC unit tests to use correctly aligned packet prefixes and correct AEAD block size assumptions.

Reviewed changes

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

File	Description
src/internal.c	Rejects inbound packets whose length violates RFC block-alignment rules before any decrypt/MAC/dispatch.
tests/unit.c	Updates packet-building helpers/fixtures and adds unit tests asserting misaligned packets are rejected (cleartext + AES-GCM).
tests/regress.c	Updates minimal packet builder to generate RFC-valid, block-aligned packets for clear transport fixtures.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1189

Scan targets checked: wolfssh-bugs, wolfssh-src

Fenrir result: Approved ✅

No new issues found in the changed files.

Advisory only — this automated result does not count as a GitHub approval.