fix: null_check hardening across wolfSSH#1092
Open
MarkAtwood wants to merge 1 commit into
Open
Conversation
- #3210 apps/wolfssh/wolfssh.c: bail out with fprintf+exit(EXIT_FAILURE) when the command buffer WMALLOC returns NULL, before the NULL cursor reaches stpcpy. Matches the existing error idiom in this function. - #6694 src/agent.c: wolfSSH_AGENT_worker returns WS_SSH_NULL_E on a NULL session instead of only setting ret, avoiding an infinite busy loop.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens NULL-handling in wolfSSH by addressing two static-analysis findings that could otherwise lead to a crash in the CLI app and an infinite loop in the agent worker.
Changes:
- Add a NULL check after
WMALLOC()inconfig_parse_command_line()to avoid passing a NULL cursor intostpcpy(). - Return immediately on NULL
WOLFSSH*inwolfSSH_AGENT_worker()to avoid an infinite busy loop.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/agent.c |
Returns WS_SSH_NULL_E immediately when ssh is NULL to prevent an infinite loop in the agent worker. |
apps/wolfssh/wolfssh.c |
Adds allocation failure handling for the command buffer to prevent a NULL dereference in command parsing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
1682
to
+1683
| if (ssh == NULL) | ||
| ret = WS_SSH_NULL_E; | ||
| return WS_SSH_NULL_E; |
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.
Two NULL-handling hardening fixes reported by Fenrir static analysis. Build-verified with
--enable-allagainst wolfSSL built--enable-all --enable-ssh(make -j8, exit 0).apps/wolfssh/wolfssh.c(config_parse_command_line) — the command-bufferWMALLOCreturn was unchecked, so a NULLcommandbecame a NULLcursorpassed tostpcpy(crash). Now bails out withfprintf(stderr, ...)+exit(EXIT_FAILURE), matching the existing error idiom in this same function.src/agent.c(wolfSSH_AGENT_worker) — on a NULL session the code only setret = WS_SSH_NULL_Ethen enteredwhile (1), which never breaks unlessret == WS_SUCCESS, causing an infinite busy loop. Now returnsWS_SSH_NULL_Eimmediately.Reported by Fenrir static analysis.