Static-analysis fixes#965
Merged
padelsbach merged 4 commits intowolfSSL:masterfrom May 8, 2026
Merged
Conversation
Issue: F-57
WS_MoveFileA allocated unicodeOldName but did not free it when the follow-up mbstowcs_s calls failed. Check the conversion result after filling unicodeOldName, and free it before returning when the sizing call for unicodeNewName fails. Affected function: WS_MoveFileA. Issue: F-50
Issue: F-214
Contributor
There was a problem hiding this comment.
Pull request overview
This PR applies a set of targeted static-analysis fixes across the wolfSSH codebase, primarily tightening bounds/error handling in packet decoding, correcting a config print bug, and addressing memory hygiene issues.
Changes:
- Harden message decoding in
src/internal.cby usingGetUint32()forUNIMPLEMENTED/DISCONNECTparsing. - Fix Windows
WS_MoveFileA()cleanup on conversion error paths to avoid leakingunicodeOldName. - Improve security hygiene by zeroing sensitive buffers before freeing in
wolfsshdpassword auth; fix client config printing ofpubKeyFile.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/internal.c | Switches parsing from raw ato32() to GetUint32() for safer bounds-checked decoding. |
| src/port.c | Frees unicodeOldName on additional error paths in WS_MoveFileA() to prevent leaks. |
| apps/wolfsshd/auth.c | Zeroes storedHashCpy prior to free to reduce sensitive-data exposure. |
| apps/wolfssh/wolfssh.c | Corrects config_print() to print pubKeyFile instead of duplicating keyFile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Replace raw ato32() with GetUint32() to check len. - Return WS_BUFFER_E on short payload instead of reading past buffer end. - DoDisconnect: parse the description and language identifier strings with GetSkip() so a truncated record returns WS_BUFFER_E instead of being treated as success. Issue: F-413
Contributor
yosuke-wolfssl
left a comment
yosuke-wolfssl
left a comment
There was a problem hiding this comment.
everything looks good
padelsbach
approved these changes
May 8, 2026
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.
Issues: F-50, F-57, F-214, F-413