wolfsftp: buffer the SFTP DATA length across partial reads

[yosuke-wolfssl]

Problem

wolfSSH_SFTP_SendReadPacket() decoded the 4-byte string length prefixing an SFTP DATA reply with a single unchecked read into a stack buffer:

byte szFlat[UINT32_SZ];                       /* uninitialized */
ret = wolfSSH_stream_read(ssh, szFlat, UINT32_SZ);
if (ret < 0) { ... }                          /* short reads fall through */
ato32(szFlat, &sz);

wolfSSH_stream_read() returns min(bufSz, available), so a short positive return is normal whenever the channel input buffer holds fewer than 4 bytes — routine when a peer's DATA reply straddles SSH packets. Only ret < 0 was checked, so 1-3 delivered bytes reached ato32(), which read the rest of szFlat as uninitialized stack (CWE-457). The decoded length was then garbage: a bogus allocation size, a spurious "Server sent more data then expected", or — as the regression test shows — a length of 0 that drops into STATE_SEND_READ_REMAINDER with a zero-byte request and fails with WS_BAD_ARGUMENT. Because szFlat was a stack local, the prefix bytes already consumed were unrecoverable, so the retry could not resynchronize. Severity: High. Closes f-8828.

Fix (src/wolfsftp.c)

STATE_SEND_READ_FTP_DATA now reads the length through wolfSSH_SFTP_buffer_read() into state->buffer instead of a raw wolfSSH_stream_read() into szFlat:

• wolfSSH_SFTP_buffer_read() accumulates short positive reads within a call, and state->buffer lives in ssh->sendReadState, so a partial prefix survives across calls.
• wolfSSH_SFTP_buffer_create(..., UINT32_SZ) is a no-op on re-entry (data != NULL && sz == UINT32_SZ), preserving idx so the retry resumes mid-prefix.
• szFlat is removed; ato32() decodes from the state buffer.

This is the same change commit 6f0cbe3f made for the sibling VERSION-header defect (f-7505), so both halves of the client read path now use one pattern. STATE_SEND_READ_FTP_DATA was the last live raw stream_read in the file without short-read handling — STATE_SEND_READ_GET_HEADER already went through SFTP_GetHeader(), and STATE_SEND_READ_REMAINDER already accumulated into state->recvSz.

Test harness (tests/unit.c)

test_SftpSendReadPacketSplit() — network-free, built on the existing SftpClientNewSession() + wolfSSH_TestChannelPutData() harness. It stages a DATA reply split inside the length prefix (1-of-4 and 2-of-4 bytes) and asserts the first call returns WS_FATAL_ERROR/WS_WANT_READ with the send-read state retained, and the retry returns the full payload intact. No new test hooks. SftpBuildData() delegates to the existing SftpBuildVersion() rather than re-packing the header.

Verification

• Negative control: test fails at the length-decode assertion without the fix, passes with it.
• make check: 11 passed, 0 failed, 1 skipped (external).
• Clean under gcc-13 -Werror across 6 configs (enable-all, zephyr-defines, sftp-only, scp-only, default, smallstack).

[Copilot]

Pull request overview

This PR fixes a high-severity short-read handling defect in the client SFTP read path by ensuring the 4-byte DATA string-length prefix is buffered across calls, preventing decoding from partially filled/uninitialized memory. It also adds a focused unit regression test that simulates a DATA reply split inside the length prefix and verifies correct WS_WANT_READ behavior and retry completion.

Changes:

• Update wolfSSH_SFTP_SendReadPacket() to read the DATA length prefix via wolfSSH_SFTP_buffer_* APIs so partial reads are accumulated safely.
• Remove the stack-based szFlat length buffer and decode the length from the persistent state buffer.
• Add a unit test that injects a split DATA reply and asserts state retention and correct payload on retry.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/wolfsftp.c	Buffer the 4-byte DATA length prefix across partial reads using persistent send-read state buffering.
tests/unit.c	Add a regression test that splits the DATA length prefix across reads and validates retry behavior/payload integrity.

---

💡 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 #1181

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.