sftp parse refresh#961
Conversation
ejohnstown
commented
Apr 30, 2026
ejohnstown
commented
- Expose Get* parsers (GetUint32, GetSize, GetStringRef, GetSkip) as WOLFSSH_API.
- Add bounds checks to ato32 reads in the four Recv* handlers called out by F-412.
- Route the remaining server Recv* handlers through GetStringRef.
- Convert SFTP_ParseAtributes_buffer to GetUint32/GetSkip and DoStatus/DoName to GetStringRef.
There was a problem hiding this comment.
Pull request overview
This PR hardens and standardizes SFTP packet parsing by switching server-side SFTP handlers away from direct ato32 reads toward the shared Get* parsing helpers, and by exporting those helpers for broader reuse.
Changes:
- Export
Get*parsing helpers (e.g.,GetUint32,GetSize,GetStringRef,GetSkip) asWOLFSSH_API. - Replace multiple SFTP
Recv*handlers’ manual length/value parsing withGetStringRef/GetUint32/GetSizebounds-checked helpers. - Refactor
SFTP_ParseAtributes_buffer()and client-sideDoStatus/DoNameparsing to use the shared parsing helpers.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| wolfssh/internal.h | Changes parsing helper declarations from WOLFSSH_LOCAL to WOLFSSH_API to export them. |
| src/wolfsftp.c | Routes many SFTP request/response parsing paths through GetStringRef/GetUint32/GetSize/GetSkip and adds bounds checks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #961
Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 1
Info (1)
Unused sz variable in Windows wolfSSH_SFTP_RecvOpen
File: src/wolfsftp.c:2225
Function: wolfSSH_SFTP_RecvOpen
Category: Dead/unreachable code
word32 sz; is declared in the Windows variant of wolfSSH_SFTP_RecvOpen but no longer referenced after this PR replaced its uses with strSz. Compilers will warn under -Wunused-variable.
Recommendation: Delete the unused word32 sz; declaration.
Referenced code: src/wolfsftp.c:2225-2227 (3 lines)
This review was generated automatically by Fenrir. Findings are non-blocking.
Convert RecvOpen, RecvWrite, RecvRead, and RecvRename (POSIX and Windows) to GetUint32, GetSize, and GetStringRef so each ato32 reads only what the buffer can supply. Issue: F-412
Convert the unflagged server Recv* handlers (RecvRealPath, RecvRMDIR, RecvMKDIR, RecvOpenDir, RecvReadDir, RecvClose, RecvRemove, RecvFSTAT, RecvSTAT, RecvLSTAT, RecvSetSTAT, RecvFSetSTAT) to GetStringRef, GetSize, and GetUint32 so all Recv* paths share one bounds-checked idiom.
Rename SFTP_ParseAtributes_buffer and SFTP_AtributesSz (and the unused SFTP_ParseAtributes) to spell "Attributes".
Replace the hand-written ato32 plus localIdx + N > maxIdx checks in SFTP_ParseAttributes_buffer with GetUint32 and GetSkip, and drop the manual length plus copy sequences in wolfSSH_SFTP_DoStatus and wolfSSH_SFTP_DoName in favor of GetStringRef so the bounds-check arithmetic lives in one place.
