Static Analysis Fixes#998
Open
ejohnstown wants to merge 2 commits into
Open
Conversation
Contributor
ejohnstown
commented
Jun 2, 2026
ejohnstown
commented
- SendKexInit: advertise ext-info-s (F-3448): Server now advertises ext-info-s per RFC 8308 Section 2.1. Splits SendExtInfo into server/client variants gated by NO_WOLFSSH_SERVER/NO_WOLFSSH_CLIENT; client path is currently a stub.
- SFTP: retry short writes (F-3880): wolfSSH_SFTP_RecvWrite loops WPWRITE until the chunk is fully written, advancing the split 64-bit offset with carry. Returns WOLFSSH_FTP_FAILURE on error or a zero return so clients see truncation instead of a silent success.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses two static-analysis-driven correctness gaps in wolfSSH’s protocol handling: (1) negotiating and advertising RFC 8308 extension-info (ext-info-c/ext-info-s) during KEX, and (2) handling partial/short writes in the SFTP write request path so a backend short write doesn’t get treated as a silent success.
Changes:
- Update KEXINIT/ext-info negotiation so servers advertise
ext-info-sandSendExtInfo()is split into server/client implementations with compile-time gating. - Update
wolfSSH_SFTP_RecvWrite()to retryWPWRITE()on short writes while advancing the 64-bit offset with carry propagation. - Minor logging/comment adjustments around the ext-info paths.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/wolfsftp.c |
Adds a retry loop for SFTP writes and advances the request offset across partial writes. |
src/internal.c |
Advertises ext-info-s on the server, refines ext-info negotiation, and splits SendExtInfo() into server/client variants. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Per RFC 8308 Section 2.1. 1. The server will now advertize that it accepts an ext-info message. 2. The client and server will send the ext-info message if requested and and allowed. The per-side send functions are guarded based on configuration. The wolfSSH client currently doesn't have anything to say in an ext-info message. Issue: F-3448
1. Loop WPWRITE until the full chunk is written, advancing the split 64-bit offset with carry. Bail out on error or a zero return and report WOLFSSH_FTP_FAILURE so clients see the truncation instead of a silent success. 2. Update the backup WPWRITE() implemented with fwrite() to parallel the behavior of the backup WPREAD(). Changed to write sz number of 1 byte objects. Issue: F-3880
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.