Conversation
Change && to || in 5 instances where public key type matching used AND instead of OR, causing WMEMCMP to be skipped when type sizes matched. Two key types with the same size but different content would incorrectly pass validation. Affected functions: DoUserAuthRequestRsaCert, DoUserAuthRequestEcc, and DoUserAuthRequestEccCert.
The while loop condition only checked that the opcode byte was in bounds (idx < modesSz) but not the 4-byte argument read by ato32(). When modesSz had a remainder of 1 mod 5 and the trailing byte was a valid opcode (1-159) rather than TTY_OP_END, ato32() would read 4 bytes past the buffer. Change the loop guard to require a full TERMINAL_MODE_SZ bytes remaining before entering the loop body.
Fix inverted WMEMCMP check that removed non-matching entries and kept matching ones. WMEMCMP returns 0 on match, so the condition was backwards. Fix head-of-list removal setting idList to NULL instead of cur->next, which dropped all remaining entries after the removed one
WMEMCMP was comparing the computed SHA-256 digest against the WOLFSSH_AGENT_ID struct pointer instead of the id field within the struct, causing key lookups by digest to never match.
The upper-bound check in the octal-to-integer conversion loop used modeOctet[0] instead of modeOctet[i], so only the first character was validated against '7'. Characters at positions 1-3 could have values above '7' without triggering an error.
In DoCheckUser, after calling auth->checkUserCb(usr) into rc, the failure check on line 1063 compared ret instead of rc against WSSHD_AUTH_FAILURE. Since ret is WOLFSSH_USERAUTH_SUCCESS at that point, the condition was always false, causing callback failures to fall through to the generic error branch with WOLFSSH_USERAUTH_FAILURE instead of returning WOLFSSH_USERAUTH_INVALID_USER.
Move the NULL validation checks inside the existing NULL guards for ssh and ssh->ctx. Previously, the check accessed ssh->ctx outside the guard, causing a NULL dereference when ssh or ssh->ctx was NULL. Also fix wolfSSH_SetTpmKey to check tpmKey instead of tpmDev.
dgarske
approved these changes
Feb 23, 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
3 participants
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.
Fix some bugs found by the wolfSSL static analyzer: