Skip to content

sftp: bound reply lengths before client allocs - #1334

Open
ejohnstown wants to merge 1 commit into
wolfSSL:masterfrom
ejohnstown:sftp-hdr
Open

ejohnstown wants to merge 1 commit into
wolfSSL:masterfrom
ejohnstown:sftp-hdr

Conversation

@ejohnstown

Copy link
Copy Markdown
Contributor

The SFTP client bounds a reply's declared length before allocating for
it. Each reply handler allocated whatever size the server's header
declared, so one 9-byte header could make the client allocate 2 GiB.

  • SFTP_GetHeader takes a limit from each caller: WOLFSSH_MAX_SFTP_PACKET
    for most replies, WOLFSSH_MAX_SFTP_NAME for NAME, the caller's buffer
    for READ; SendWritePacket keeps that error instead of a generic one.
  • New regress test drives all eleven client calls at and past the bound.

Issue: F-10598, F-13430, F-14433

SFTP_GetHeader now takes a limit from its caller and refuses a declared
body over it, or one too short to hold the header, with WS_BUFFER_E.
Every client reply handler allocated a buffer of the peer-declared size
first, so one 9-byte header could make the client allocate 2 GiB.

- STATUS, ATTRS and HANDLE replies are held to WOLFSSH_MAX_SFTP_PACKET
- NAME keeps WOLFSSH_MAX_SFTP_NAME; DoName's own check moves into
  the call
- a READ reply may also be as large as the caller's buffer plus its
  length prefix
- SendWritePacket keeps an error already set instead of
  WS_SFTP_BAD_HEADER
- regress test drives all eleven client calls at and past the bound

Issue: F-10598, F-13430, F-14433
Copilot AI balanced review requested due to automatic review settings October 20, 2026 22:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants