Conversation
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1258
Error: RuntimeError
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1258
Error: RuntimeError
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1258
Error: RuntimeError
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1258
Error: RuntimeError
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1258
Scan targets checked: wolfssh-src, wolfssh-bugs
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
| WFREE(ret, heap, DYNTYPE_SSHD); | ||
| ret = NULL; | ||
| /* Set *err to WS_MEMORY_E on alloc failure, or WS_FATAL_ERROR on conversion failure. */ | ||
| static char* _convertHelper(WCHAR* in, void* heap, int* err) { |
There was a problem hiding this comment.
Convention: The opening brace for a function should be on its own line.
| char* ret = NULL; | ||
| size_t needed = 0; | ||
|
|
||
| /* Query exact size needed for multi-byte ACPs. Avoids zero-size for empty args. */ |
There was a problem hiding this comment.
Convention: Lines are supposed to wrap before column 80. Just kick the second sentence to the next line.
| /* No WOLFSSH_AGENT branch: only RSA certs support agent signing. */ | ||
| if (authData->sf.publicKey.privateKey == NULL || | ||
| authData->sf.publicKey.privateKeySz == 0) { | ||
| /* Avoid misleading ASN error for cert-store-only clients without in-memory key. */ |
There was a problem hiding this comment.
Convention: Lines are supposed to wrap before column 80. Just split it somewhere.
| &idx, &keySig->ks.ecc.key, | ||
| authData->sf.publicKey.privateKeySz); | ||
| } | ||
| /* No WOLFSSH_AGENT branch: only RSA certs support agent signing. */ |
There was a problem hiding this comment.
Reword. Among the certificate key types, only RSA implements agent signing. Plain ECDSA keys do use the agent, in the non-certificate ECC path.
| } | ||
| else | ||
| #endif /* WOLFSSH_WINDOWS_CERT_STORE */ | ||
| /* No WOLFSSH_AGENT branch: only RSA certs support agent signing. */ |
There was a problem hiding this comment.
Reword. Among the certificate key types, only RSA implements agent signing. Plain ECDSA keys do use the agent, in the non-certificate ECC path.
| } | ||
| else { | ||
| size_t numConv = 0; | ||
| if (wcstombs_s(&numConv, ret, needed, in, needed - 1) != 0) { |
There was a problem hiding this comment.
needed is already big enough. You don't need to subtract 1 here. The sizing query returns the size including the terminating null and that's what you allocate, so pass needed for the count too.
4 participants
Reworked the wide-char to multibyte conversion so the exact buffer size needed is used. Removed dead ECC agent code, as only RSA supports agent signing. Added an intialized to 0 to avoid use-of-unitialized value warnings.