Skip to content

Commit d3d3e7e

Browse files
ejohnstownphilljj
authored andcommitted
Fix the cppcheck findings
Four sites re-tested an unchanged ret after a WOLFSSH_SMALL_STACK allocation, which cppcheck reports as identicalInnerCondition because the allocation that can change ret is compiled out in the default build. Test the allocated pointer instead; that is what the check is guarding. - certman.c, keygen.c and wolfsshd/auth.c: check the DecodedCert, MlDsaKey and DecodedCert pointers. - internal.c CompositeEccSign: give the fixed-buffer build pointer aliases so both builds have the same shape, then check the r/s pointers. - port.c: initialize fileHandle, which is only assigned when mbstowcs_s succeeds. - wolfsftp.c: zero localTime before WLOCALTIME, which is a per-port macro.
1 parent a98fbdd commit d3d3e7e

6 files changed

Lines changed: 16 additions & 8 deletions

File tree

apps/wolfsshd/auth.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2695,8 +2695,8 @@ static int RequestAuthentication(WS_UserAuthData* authData,
26952695
DecodedCert* dCert = &sdCert;
26962696
#endif
26972697

2698-
/* ret is still success unless the allocation above failed */
2699-
if (ret == WOLFSSH_USERAUTH_SUCCESS) {
2698+
/* NULL only when the small-stack allocation above failed */
2699+
if (dCert != NULL) {
27002700
wc_InitDecodedCert(dCert, authData->sf.publicKey.publicKey,
27012701
authData->sf.publicKey.publicKeySz, NULL);
27022702
if (wc_ParseCert(dCert, CERT_TYPE, NO_VERIFY, NULL) != 0) {

src/certman.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,8 @@ int wolfSSH_CERTMAN_VerifyCerts_buffer(WOLFSSH_CERTMAN* cm,
404404
}
405405
#endif
406406

407-
if (ret == WS_SUCCESS) {
407+
/* NULL only when the small-stack allocation above failed */
408+
if (decoded != NULL) {
408409
wc_InitDecodedCert(decoded, certLoc[0], certLen[0], cm->heap);
409410
if (wc_ParseCert(decoded, WOLFSSL_FILETYPE_ASN1, NO_VERIFY, cm->cm)
410411
!= 0) {

src/internal.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21926,11 +21926,14 @@ static int CompositeEccSign(void* key, WC_RNG* rng, void* heap,
2192621926
ret = WS_MEMORY_E;
2192721927
}
2192821928
#else
21929-
byte rBuf[MAX_ECC_BYTES + ECC_MAX_PAD_SZ];
21930-
byte sBuf[MAX_ECC_BYTES + ECC_MAX_PAD_SZ];
21929+
byte rStore[MAX_ECC_BYTES + ECC_MAX_PAD_SZ];
21930+
byte sStore[MAX_ECC_BYTES + ECC_MAX_PAD_SZ];
21931+
byte* rBuf = rStore;
21932+
byte* sBuf = sStore;
2193121933
#endif
2193221934

21933-
if (ret == 0) {
21935+
/* NULL only when a small-stack allocation above failed */
21936+
if (rBuf != NULL && sBuf != NULL) {
2193421937
ret = wc_ecc_sig_to_rs(asnSig, asnSigSz, rBuf, &rSz, sBuf, &sSz);
2193521938
}
2193621939
if (ret == 0) {

src/keygen.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,8 @@ int wolfSSH_MakeMlDsaKey(byte* out, word32 outSz, word32 level)
312312
}
313313
#endif
314314

315-
if (ret == WS_SUCCESS) {
315+
/* NULL only when the small-stack allocation above failed */
316+
if (key != NULL) {
316317
if (wc_MlDsaKey_Init(key, NULL, INVALID_DEVID) != 0)
317318
ret = WS_CRYPTO_FAILED;
318319
else {

src/port.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ void* WS_CreateFileA(const char* fileName, unsigned long desiredAccess,
250250
unsigned long shareMode, unsigned long creationDisposition,
251251
unsigned long flags, void* heap)
252252
{
253-
HANDLE fileHandle;
253+
HANDLE fileHandle = INVALID_HANDLE_VALUE;
254254
wchar_t* unicodeFileName;
255255
size_t unicodeFileNameSz = 0;
256256
size_t returnSz = 0;

src/wolfsftp.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3151,6 +3151,9 @@ static int SFTP_CreateLongName(WS_SFTPNAME* name)
31513151
mtime = (time_t)atr->mtime;
31523152

31533153
#if defined(WLOCALTIME)
3154+
/* WLOCALTIME is a per-port macro; zero first so nothing reads a stale
3155+
* struct if a port leaves it untouched. */
3156+
WMEMSET(&localTime, 0, sizeof(localTime));
31543157
if (!WLOCALTIME(&mtime, &localTime)) {
31553158
return WS_MEMORY_E;
31563159
}

0 commit comments

Comments
 (0)