Repository navigation
wolfsshd: fix two auth.c defects - #1389
Conversation
SetupUserTokenWin frees its WCHAR copy of the user name in the common cleanup with authInfo and the LSA handle, so a public key probe no longer leaks it on Windows. Issue: F-8854
CheckPasswordPAM takes the WOLFSSHD_AUTH argument that CallbackCheckPassword requires, so a --with-pam build compiles again. It is still a stub and returns WSSHD_AUTH_FAILURE for every password. Issue: F-11676
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and found no bugs. Both fixes look correct, but the file is wolfsshd's authentication code, so a human look is still worthwhile.
What was reviewed:
- The new
CheckPasswordPAMsignature matchesCallbackCheckPasswordin auth.h, which it is assigned to at auth.c:3305.WSSHD_AUTH_FAILUREis defined as 0, so the stub returns the same value as before (it still refuses every password). The change only makes the intent explicit. usrWinSetupUserTokenWinstarts as NULL and is allocated withWMALLOC(..., NULL, DYNTYPE_SSHD), which matches the newWFREE. Every path reaches the single cleanup block and nothing else frees it, so it can't be freed twice.- Not done: a
--with-pamor Windows build. This review only read the code.
Extended reasoning...
The change touches only apps/wolfsshd/auth.c: the PAM password stub gets a corrected signature, and a Windows public-key token helper stops leaking a buffer. Both are in the SSH daemon's authentication path, which is security-sensitive, and the approval guidelines exclude auth code from auto-approval. The stub still returns 0 (WSSHD_AUTH_FAILURE) as before, so no auth decision changes. The free matches the allocation's heap and type and runs exactly once. Neither changed path is compiled in a default Linux build, so neither was build-checked here.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1389
Scan targets checked: wolfssh-src, wolfssh-bugs
Coverage: 1 of 1 in-scope changed file(s) opened by the reviewer
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Review tier: Max
Issue: F-8854, F-11676