Static Analysis Fixes#988
Merged
Merged
Conversation
Contributor
ejohnstown
commented
May 20, 2026
ejohnstown
commented
- wolfssh: fix leak of pre-allocated config->user and copy the argv user name into a heap buffer (F-4102).
- wolfsshd: expose CheckAuthKeysLine under WOLFSSHD_UNIT_TEST and add coverage for match, mismatched key, and same-length last-byte-differs cases (F-4107).
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses static-analysis findings in the wolfSSH client and wolfSSHD by fixing config->user ownership/lifetime on the client side and adding unit-test exposure plus coverage for authorized-keys line comparison logic on the server side.
Changes:
- wolfssh client: copy
-l <user>into heap memory and free any priorconfig->uservalue to avoid leaks/invalid frees. - wolfsshd: expose
CheckAuthKeysLine()underWOLFSSHD_UNIT_TESTand add unit tests covering match and key-mismatch scenarios (including same-length last-byte difference). - test harness: adjust test logging output and add Base64 helpers for generating authorized_keys-style lines.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/wolfsshd/test/test_configuration.c | Adds Base64-backed test cases for CheckAuthKeysLine() and updates logging helper. |
| apps/wolfsshd/auth.h | Declares CheckAuthKeysLine() for unit tests under WOLFSSHD_UNIT_TEST. |
| apps/wolfsshd/auth.c | Makes CheckAuthKeysLine() non-static under WOLFSSHD_UNIT_TEST to enable direct test coverage. |
| apps/wolfssh/wolfssh.c | Fixes -l option handling to avoid leaking/freeing non-owned config->user pointers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1. Fix where an already allocated config->user is leaked. 2. Copy the argv user name into a heap buffer. Issue: F-4102
- Expose CheckAuthKeysLine under WOLFSSHD_UNIT_TEST so tests can link it. - Add test_CheckAuthKeysLine covering match, different key, and same-length key differing only in the last byte. - Route the test Log() to stderr instead of an unused local buffer. Issue: F-4107
padelsbach
approved these changes
May 20, 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.