Static analysis fixes#969
Merged
Merged
Conversation
Contributor
ejohnstown
commented
May 11, 2026
ejohnstown
commented
- F-3444: ClientPublicKeyCheck() returned WS_SUCCESS when the user declined to add an unknown host key, silently accepting it. Now sets ret = -1 in the decline branch to abort the handshake.
- F-3445: Ed25519 user-auth verify overwrote failure results, so tampered signatures could pass. verify_msg_final errors now map to WS_CRYPTO_FAILED and status == 0 to WS_ED25519_E. Added a test shim and unit test for valid and tampered signatures.
- F-3682: SFTP_RecvRead's bound check could underflow letting a malformed request bypass the size check and overflow WMALLOC. Now compares strSz against WOLFSSH_MAX_SFTP_RECV directly, in both POSIX and Windows variants.
ClientPublicKeyCheck() prompted to add an unknown server's key to known_hosts but, if the user declined, left ret at 0 and returned WS_SUCCESS, silently accepting the unverified host. Set ret = -1 in the else branch to mirror the otherMatch case and abort the handshake when the user does not confirm. Issue: F-3444
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes multiple static-analysis-identified correctness/security issues across the wolfSSH client, Ed25519 public-key auth verification, and SFTP READ request handling.
Changes:
- Abort client handshake when the user declines to trust/add an unknown host key (known_hosts prompt path).
- Fix Ed25519 USERAUTH signature verification so verification failures can’t be overwritten into success; add an internal test shim + unit test for valid vs tampered signatures.
- Harden SFTP READ length bounds checking to avoid underflow-based bypass of size limits.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| wolfssh/internal.h | Adds internal-test API declaration for driving Ed25519 auth verification directly. |
| src/internal.c | Fixes Ed25519 verify return mapping and adds a WOLFSSH_TEST_INTERNAL shim for unit testing. |
| tests/unit.c | Adds deterministic Ed25519 signature verification unit test (positive + tampered negative case). |
| src/wolfsftp.c | Replaces underflow-prone SFTP READ bounds check with a direct maximum-size check (POSIX + Windows). |
| apps/wolfssh/common.c | Ensures declining an unknown host key causes callback failure (prevents silent acceptance). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Map ed25519 verify_msg_final errors to WS_CRYPTO_FAILED and status==0 to WS_ED25519_E (no longer overwritten) - Expose DoUserAuthRequestEd25519 via test shim - Add unit test covering valid and tampered signatures Issue: F-3445
ejohnstown
force-pushed
the
static-fixes-2
branch
from
0652977 to
7fcf4ca
Compare
- Replace `strSz > maxSz - WOLFSSH_SFTP_HEADER - idx` with `strSz > WOLFSSH_MAX_SFTP_RW` to prevent unsigned wraparound and downstream WMALLOC size overflow. - Apply to both POSIX and Windows variants. Issue: F-3682
ejohnstown
force-pushed
the
static-fixes-2
branch
from
7fcf4ca to
60c6b38
Compare
padelsbach
approved these changes
May 12, 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.