Skip to content

Commit 4696a91

Browse files
committed
Add cppcheck test to GitHub actions
Found and fixed: * Fix typos in Renesas demo * Fix uninitialized variable reads * Fix redundant condition * Fix argument checks * Fix some null ptr dereferences * Fix ambiguous statement
1 parent 759bcbd commit 4696a91

File tree

9 files changed

+125
-80
lines changed

9 files changed

+125
-80
lines changed

.github/workflows/cppcheck.yml

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
name: Cppcheck Test
2+
3+
on:
4+
push:
5+
branches: [ '*' ]
6+
pull_request:
7+
branches: [ '*' ]
8+
9+
jobs:
10+
run_cppcheck:
11+
name: Cppcheck
12+
runs-on: ubuntu-24.04
13+
steps:
14+
- uses: actions/checkout@v4
15+
16+
- name: Install cppcheck
17+
if: always()
18+
run: sudo apt-get install cppcheck
19+
20+
- name: Run CppCheck
21+
id: cpp_check_run
22+
if: always()
23+
run: >
24+
cppcheck
25+
-UWSCPFILEHDR -UXSNPRINTF
26+
-DLIBWOLFSSH_VERSION_STRING='""'
27+
--enable='warning,portability'
28+
--std=c99
29+
--force
30+
--check-level=exhaustive
31+
--error-exitcode=2
32+
--library=std.cfg
33+
--inline-suppr
34+
-j4
35+
-q
36+
.
37+
3>&1 1>&2 2>&3 | tee cppcheck.txt
38+
39+
- name: Upload cppcheck results as artifact
40+
if: always()
41+
uses: actions/upload-artifact@v4
42+
with:
43+
name: wolfssh-${{ github.sha }}-cppcheck_results.txt
44+
path: cppcheck.txt

apps/wolfsshd/auth.c

Lines changed: 50 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ static int CheckPasswordHashUnix(const char* input, char* stored)
320320
}
321321

322322
/* empty password case */
323-
if (stored[0] == 0 && WSTRLEN(input) == 0) {
323+
if (ret == WSSHD_AUTH_SUCCESS && stored[0] == 0 && WSTRLEN(input) == 0) {
324324
wolfSSH_Log(WS_LOG_INFO,
325325
"[SSHD] User logged in with empty password");
326326
return ret;
@@ -1206,49 +1206,47 @@ static int RequestAuthentication(WS_UserAuthData* authData,
12061206
}
12071207
#endif
12081208

1209-
if (ret == WOLFSSH_USERAUTH_SUCCESS) {
1210-
/* if this is a certificate and no specific authorized keys file has
1211-
* been set then rely on CA to have verified the cert */
1212-
if (authData->sf.publicKey.isCert &&
1213-
!wolfSSHD_ConfigGetAuthKeysFileSet(authCtx->conf)) {
1214-
wolfSSH_Log(WS_LOG_INFO,
1215-
"[SSHD] Relying on CA for public key check");
1216-
#ifdef WIN32
1217-
/* Still need to get users token on Windows */
1218-
rc = SetupUserTokenWin(usr, &authData->sf.publicKey,
1219-
wolfSSHD_ConfigGetUserCAKeysFile(authCtx->conf), authCtx);
1220-
if (rc == WSSHD_AUTH_SUCCESS) {
1221-
wolfSSH_Log(WS_LOG_INFO, "[SSHD] Got users token ok.");
1222-
ret = WOLFSSH_USERAUTH_SUCCESS;
1223-
}
1224-
else {
1225-
wolfSSH_Log(WS_LOG_ERROR,
1226-
"[SSHD] Error getting users token.");
1227-
ret = WOLFSSH_USERAUTH_FAILURE;
1228-
}
1229-
#else
1209+
/* if this is a certificate and no specific authorized keys file has
1210+
* been set then rely on CA to have verified the cert */
1211+
if (authData->sf.publicKey.isCert &&
1212+
!wolfSSHD_ConfigGetAuthKeysFileSet(authCtx->conf)) {
1213+
wolfSSH_Log(WS_LOG_INFO,
1214+
"[SSHD] Relying on CA for public key check");
1215+
#ifdef WIN32
1216+
/* Still need to get users token on Windows */
1217+
rc = SetupUserTokenWin(usr, &authData->sf.publicKey,
1218+
wolfSSHD_ConfigGetUserCAKeysFile(authCtx->conf), authCtx);
1219+
if (rc == WSSHD_AUTH_SUCCESS) {
1220+
wolfSSH_Log(WS_LOG_INFO, "[SSHD] Got users token ok.");
12301221
ret = WOLFSSH_USERAUTH_SUCCESS;
1231-
#endif
12321222
}
12331223
else {
1234-
/* if not a certificate then parse through authorized key file */
1235-
rc = authCtx->checkPublicKeyCb(usr, &authData->sf.publicKey,
1236-
wolfSSHD_ConfigGetUserCAKeysFile(authCtx->conf),
1237-
authCtx);
1238-
if (rc == WSSHD_AUTH_SUCCESS) {
1239-
wolfSSH_Log(WS_LOG_INFO, "[SSHD] Public key ok.");
1240-
ret = WOLFSSH_USERAUTH_SUCCESS;
1241-
}
1242-
else if (rc == WSSHD_AUTH_FAILURE) {
1243-
wolfSSH_Log(WS_LOG_INFO,
1244-
"[SSHD] Public key not authorized.");
1245-
ret = WOLFSSH_USERAUTH_INVALID_PUBLICKEY;
1246-
}
1247-
else {
1248-
wolfSSH_Log(WS_LOG_ERROR,
1249-
"[SSHD] Error checking public key.");
1250-
ret = WOLFSSH_USERAUTH_FAILURE;
1251-
}
1224+
wolfSSH_Log(WS_LOG_ERROR,
1225+
"[SSHD] Error getting users token.");
1226+
ret = WOLFSSH_USERAUTH_FAILURE;
1227+
}
1228+
#else
1229+
ret = WOLFSSH_USERAUTH_SUCCESS;
1230+
#endif
1231+
}
1232+
else {
1233+
/* if not a certificate then parse through authorized key file */
1234+
rc = authCtx->checkPublicKeyCb(usr, &authData->sf.publicKey,
1235+
wolfSSHD_ConfigGetUserCAKeysFile(authCtx->conf),
1236+
authCtx);
1237+
if (rc == WSSHD_AUTH_SUCCESS) {
1238+
wolfSSH_Log(WS_LOG_INFO, "[SSHD] Public key ok.");
1239+
ret = WOLFSSH_USERAUTH_SUCCESS;
1240+
}
1241+
else if (rc == WSSHD_AUTH_FAILURE) {
1242+
wolfSSH_Log(WS_LOG_INFO,
1243+
"[SSHD] Public key not authorized.");
1244+
ret = WOLFSSH_USERAUTH_INVALID_PUBLICKEY;
1245+
}
1246+
else {
1247+
wolfSSH_Log(WS_LOG_ERROR,
1248+
"[SSHD] Error checking public key.");
1249+
ret = WOLFSSH_USERAUTH_FAILURE;
12521250
}
12531251
}
12541252
}
@@ -1545,23 +1543,23 @@ int wolfSSHD_AuthReducePermissions(WOLFSSHD_AUTH* auth)
15451543
byte flag = 0;
15461544
int ret = WS_SUCCESS;
15471545

1546+
if (!auth) {
1547+
return WS_BAD_ARGUMENT;
1548+
}
1549+
15481550
flag = wolfSSHD_ConfigGetPrivilegeSeparation(auth->conf);
15491551
#ifndef WIN32
15501552
if (flag == WOLFSSHD_PRIV_SEPARAT || flag == WOLFSSHD_PRIV_SANDBOX) {
15511553
wolfSSH_Log(WS_LOG_INFO, "[SSHD] Lowering permissions level");
1552-
if (auth) {
1553-
if (setegid(auth->gid) != 0) {
1554-
wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error setting sshd gid");
1555-
ret = WS_FATAL_ERROR;
1556-
}
15571554

1558-
if (seteuid(auth->uid) != 0) {
1559-
wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error setting sshd uid");
1560-
ret = WS_FATAL_ERROR;
1561-
}
1555+
if (setegid(auth->gid) != 0) {
1556+
wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error setting sshd gid");
1557+
ret = WS_FATAL_ERROR;
15621558
}
1563-
else {
1564-
ret = WS_BAD_ARGUMENT;
1559+
1560+
if (seteuid(auth->uid) != 0) {
1561+
wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error setting sshd uid");
1562+
ret = WS_FATAL_ERROR;
15651563
}
15661564
}
15671565
#endif

examples/portfwd/portfwd.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ THREAD_RETURN WOLFSSH_THREAD portfwd_worker(void* args)
337337
" * password: %s\n"
338338
" * forward from: %s:%u\n"
339339
" * forward to: %s:%u\n",
340-
host, port, username, password,
340+
host, port, username, password ? password : "",
341341
fwdFromHost, fwdFromPort,
342342
fwdToHost, fwdToPort);
343343

ide/Renesas/cs+/demo_server/wolfssh_demo.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,11 +125,11 @@ static int dump_stats(thread_ctx_t* ctx)
125125

126126
wolfSSH_GetStats(ctx->ssh, &txCount, &rxCount, &seq, &peerSeq);
127127

128-
printf(stats,
128+
sprintf(stats,
129129
"Statistics for Thread #%u:\r\n"
130130
" txCount = %u\r\n rxCount = %u\r\n"
131131
" seq = %u\r\n peerSeq = %u\r\n",
132-
0, txCount, rxCount, seq, peerSeq);
132+
(word32)0, txCount, rxCount, seq, peerSeq);
133133
statsSz = (word32)strlen(stats);
134134

135135
fprintf(stderr, "%s", stats);
@@ -648,4 +648,4 @@ void abort(void)
648648
{
649649

650650
}
651-
#endif
651+
#endif

src/agent.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1303,7 +1303,10 @@ static int DoUnimplemented(WOLFSSH_AGENT_CTX* agent,
13031303
ret = WS_BAD_ARGUMENT;
13041304

13051305
WOLFSSH_UNUSED(buf);
1306-
DUMP(buf + *idx, len);
1306+
1307+
if (ret == WS_SUCCESS) {
1308+
DUMP(buf + *idx, len);
1309+
}
13071310

13081311
/* Just skip the message. */
13091312
if (ret == WS_SUCCESS) {
@@ -1498,11 +1501,12 @@ WOLFSSH_AGENT_CTX* wolfSSH_AGENT_new(void* heap)
14981501

14991502
void wolfSSH_AGENT_free(WOLFSSH_AGENT_CTX* agent)
15001503
{
1501-
void* heap = agent->heap;
1504+
void* heap = NULL;
15021505

15031506
WLOG(WS_LOG_AGENT, "Entering wolfSSH_AGENT_free()");
15041507

15051508
if (agent != NULL) {
1509+
heap = agent->heap;
15061510
if (agent->msg != NULL)
15071511
WFREE(agent->msg, agent->heap, DYNTYPE_AGENT_BUFFER);
15081512
wc_FreeRng(&agent->rng);

src/internal.c

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1329,7 +1329,6 @@ int IdentifyAsn1Key(const byte* in, word32 inSz, int isPrivate, void* heap)
13291329
}
13301330
#endif /* WOLFSSH_NO_ECDSA */
13311331
#if !defined(WOLFSSH_NO_ED25519)
1332-
if (key != NULL) {
13331332
if (key->keySigId == ID_UNKNOWN) {
13341333
idx = 0;
13351334
ret = wc_ed25519_init_ex(&key->ks.ed25519.key, heap, INVALID_DEVID);
@@ -1351,7 +1350,6 @@ int IdentifyAsn1Key(const byte* in, word32 inSz, int isPrivate, void* heap)
13511350

13521351
wc_ed25519_free(&key->ks.ed25519.key);
13531352
}
1354-
}
13551353
#endif /* WOLFSSH_NO_ED25519 */
13561354

13571355
if (key->keySigId == ID_UNKNOWN) {
@@ -1623,8 +1621,7 @@ static int GetOpenSshKey(WS_KeySignature *key,
16231621
byte keyId;
16241622

16251623
idx = 0;
1626-
if (ret == WS_SUCCESS)
1627-
ret = GetUint32(&check1, str, strSz, &subIdx); /* checkint 1 */
1624+
ret = GetUint32(&check1, str, strSz, &subIdx); /* checkint 1 */
16281625
if (ret == WS_SUCCESS)
16291626
ret = GetUint32(&check2, str, strSz, &subIdx); /* checkint 2 */
16301627
if (ret == WS_SUCCESS) {
@@ -2815,7 +2812,7 @@ int ChannelAppend(WOLFSSH* ssh, WOLFSSH_CHANNEL* channel)
28152812
int ChannelRemove(WOLFSSH* ssh, word32 channel, byte peer)
28162813
{
28172814
int ret = WS_SUCCESS;
2818-
WOLFSSH_CHANNEL* list;
2815+
WOLFSSH_CHANNEL* list = NULL;
28192816

28202817
WLOG(WS_LOG_DEBUG, "Entering ChannelRemove()");
28212818

@@ -6380,9 +6377,7 @@ static int DoUserAuthRequestRsa(WOLFSSH* ssh, WS_UserAuthData_PublicKey* pk,
63806377
const byte* e = NULL;
63816378
word32 eSz = 0;
63826379

6383-
if (ret == WS_SUCCESS) {
6384-
ret = GetMpint(&eSz, &e, pk->publicKey, pk->publicKeySz, &i);
6385-
}
6380+
ret = GetMpint(&eSz, &e, pk->publicKey, pk->publicKeySz, &i);
63866381

63876382
if (ret == WS_SUCCESS) {
63886383
ret = GetMpint(&nSz, &n, pk->publicKey, pk->publicKeySz, &i);
@@ -13157,6 +13152,12 @@ static int BuildUserAuthRequestEcc(WOLFSSH* ssh,
1315713152
byte* checkData = NULL;
1315813153
word32 checkDataSz = 0;
1315913154

13155+
if (ssh == NULL || output == NULL || idx == NULL || authData == NULL ||
13156+
sigStart == NULL || keySig == NULL) {
13157+
ret = WS_BAD_ARGUMENT;
13158+
return ret;
13159+
}
13160+
1316013161
#ifdef WOLFSSH_SMALL_STACK
1316113162
r_ptr = (byte*)WMALLOC(rSz, ssh->ctx->heap, DYNTYPE_BUFFER);
1316213163
s_ptr = (byte*)WMALLOC(sSz, ssh->ctx->heap, DYNTYPE_BUFFER);
@@ -13172,12 +13173,6 @@ static int BuildUserAuthRequestEcc(WOLFSSH* ssh,
1317213173
sig_ptr = sig_s;
1317313174
#endif
1317413175

13175-
if (ssh == NULL || output == NULL || idx == NULL || authData == NULL ||
13176-
sigStart == NULL || keySig == NULL) {
13177-
ret = WS_BAD_ARGUMENT;
13178-
return ret;
13179-
}
13180-
1318113176
begin = *idx;
1318213177

1318313178
if (ret == WS_SUCCESS) {

src/port.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ static const char* TrimFileName(const char* f, size_t* fSz)
181181
{
182182
if (f != NULL && fSz != NULL && *fSz >= 3 && f[0] == '/' && f[2] == ':') {
183183
f++;
184-
*fSz--;
184+
(*fSz)--;
185185
}
186186
return f;
187187
}
@@ -224,7 +224,7 @@ void* WS_CreateFileA(const char* fileName, unsigned long desiredAccess,
224224
void* WS_FindFirstFileA(const char* fileName,
225225
char* realFileName, size_t realFileNameSz, int* isDir, void* heap)
226226
{
227-
HANDLE findHandle;
227+
HANDLE findHandle = NULL;
228228
WIN32_FIND_DATAW findFileData;
229229
wchar_t* unicodeFileName;
230230
size_t unicodeFileNameSz = 0;
@@ -269,7 +269,7 @@ int WS_FindNextFileA(void* findHandle,
269269
{
270270
BOOL success;
271271
WIN32_FIND_DATAW findFileData;
272-
errno_t error;
272+
errno_t error = 0;
273273

274274
success = FindNextFileW((HANDLE)findHandle, &findFileData);
275275

src/ssh.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2755,15 +2755,19 @@ int wolfSSH_ChannelSend(WOLFSSH_CHANNEL* channel,
27552755
{
27562756
int bytesTxd = 0;
27572757

2758+
if (channel == NULL || buf == NULL) {
2759+
WLOG(WS_LOG_DEBUG, "Entering wolfSSH_ChannelSend() with bad argument");
2760+
return WS_BAD_ARGUMENT;
2761+
}
2762+
27582763
WLOG(WS_LOG_DEBUG, "Entering wolfSSH_ChannelSend(), ID = %d, peerID = %d",
27592764
channel->channel, channel->peerChannel);
27602765

27612766
#ifdef DEBUG_WOLFSSH
27622767
DumpOctetString(buf, bufSz);
27632768
#endif
2764-
if (channel == NULL || buf == NULL)
2765-
bytesTxd = WS_BAD_ARGUMENT;
2766-
else if (!channel->openConfirmed) {
2769+
2770+
if (!channel->openConfirmed) {
27672771
WLOG(WS_LOG_DEBUG, "Channel not confirmed yet.");
27682772
bytesTxd = WS_CHANNEL_NOT_CONF;
27692773
}

src/wolfsftp.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3647,7 +3647,7 @@ int wolfSSH_SFTP_RecvRead(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
36473647
word32 ofst[2] = {0, 0};
36483648

36493649
byte* out;
3650-
word32 outSz;
3650+
word32 outSz = 0;
36513651

36523652
char* res = NULL;
36533653
char err[] = "Read File Error";
@@ -3747,7 +3747,7 @@ int wolfSSH_SFTP_RecvRead(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
37473747
word32 idx = 0;
37483748

37493749
byte* out;
3750-
word32 outSz;
3750+
word32 outSz = 0;
37513751

37523752
char* res = NULL;
37533753
char err[] = "Read File Error";

0 commit comments

Comments
 (0)