Static analysis fixes#976
Merged
Merged
Conversation
Contributor
ejohnstown
commented
May 12, 2026
ejohnstown
commented
- Pin expectMsgId to MSGID_KEXDH_GEX_INIT after KEXDH_GEX_GROUP; null-check ssh->handshake (F-3668).
- Reject ParseRFC6187 name lengths that would overflow when added to sizeof(word32), preventing OOB reads (F-1277).
- ForceZero the lastBlock stack buffers on both remainder paths in GenerateKey (F-2494).
- ForceZero newKey before free in DoPemKey/DoOpenSshKey and the PEM read buffer in wolfSSH_ReadKey_file (F-2885, F-3211, F-3212).
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Addresses multiple static-analysis findings by tightening bounds checks, hardening handshake state transitions, and zeroing sensitive buffers before free/return.
Changes:
- Harden key parsing/reading paths by tracking buffer sizes and ForceZero’ing sensitive allocations before freeing.
- Add ForceZero on stack
lastBlockbuffers in both remainder paths ofGenerateKey. - Add RFC6187 name-length overflow protection and tighten KEXDH_GEX message expectation handling (incl.
ssh->handshakenull-check).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/ssh.c | Securely clear key/file buffers and adjust allocation sizing for PEM/OpenSSH key parsing. |
| src/internal.c | Zero sensitive stack buffers in key derivation; harden KEXDH_GEX group send flow and expected message tracking. |
| apps/wolfssh/common.c | Add bounds check to prevent RFC6187 name-length overflow/OOB reads. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
padelsbach
reviewed
May 12, 2026
padelsbach
reviewed
May 12, 2026
padelsbach
previously approved these changes
May 12, 2026
ejohnstown
force-pushed
the
static-fixes-3
branch
from
b8b6239 to
155fff7
Compare
padelsbach
approved these changes
May 12, 2026
- ForceZero newKey before WFREE in DoPemKey/DoOpenSshKey, covering both locally-allocated and caller-supplied buffers. - ForceZero PEM file buffer after read in wolfSSH_ReadKey_file - Track newKeySz for caller-supplied buffer path Issue: F-2885, F-3211, F-3212
- Add ForceZero on lastBlock in both remainder paths - Prevents leftover key-derivation material on the stack Issue: F-2494
- Reject name lengths that would overflow m when added to sizeof(word32), preventing OOB reads on later checks. Issue: F-1277
- Set expectMsgId to MSGID_KEXDH_GEX_INIT so the server rejects any other KEX message at this stage (RFC 4419 sec 3). - Reject NULL ssh->handshake in the entry check now that the success path dereferences it. Issue: F-3668
ejohnstown
force-pushed
the
static-fixes-3
branch
from
155fff7 to
6c394c1
Compare
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.