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
wolfSSL · Feb 5, 2025 · d108c69 · d108c69
1 parent 759bcbd
commit d108c69
Show file tree

Hide file tree

Showing 9 changed files with 128 additions and 81 deletions.
diff --git a/.github/workflows/cppcheck.yml b/.github/workflows/cppcheck.yml
@@ -0,0 +1,44 @@
+name: Cppcheck Test
+
+on:
+  push:
+    branches: [ '*' ]
+  pull_request:
+    branches: [ '*' ]
+
+jobs:
+  run_cppcheck:
+    name: Cppcheck
+    runs-on: ubuntu-24.04
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Install cppcheck
+        if: always()
+        run: sudo apt-get install cppcheck
+
+      - name: Run CppCheck
+        id: cpp_check_run
+        if: always()
+        run: >
+          cppcheck
+          -UWSCPFILEHDR -UXSNPRINTF
+          -DLIBWOLFSSH_VERSION_STRING='""'
+          --enable='warning,portability'
+          --std=c99
+          --force
+          --check-level=exhaustive
+          --error-exitcode=2
+          --library=std.cfg
+          --inline-suppr
+          -j4
+          -q
+          .
+          3>&1 1>&2 2>&3 | tee cppcheck.txt
+
+      - name: Upload cppcheck results as artifact
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: wolfssh-${{ github.sha }}-cppcheck_results.txt
+          path: cppcheck.txt
diff --git a/apps/wolfsshd/auth.c b/apps/wolfsshd/auth.c
@@ -320,7 +320,7 @@ static int CheckPasswordHashUnix(const char* input, char* stored)
     }
 
     /* empty password case */
-    if (stored[0] == 0 && WSTRLEN(input) == 0) {
+    if (ret == WSSHD_AUTH_SUCCESS && stored[0] == 0 && WSTRLEN(input) == 0) {
         wolfSSH_Log(WS_LOG_INFO,
                     "[SSHD] User logged in with empty password");
         return ret;
@@ -1206,49 +1206,47 @@ static int RequestAuthentication(WS_UserAuthData* authData,
         }
     #endif
 
-        if (ret == WOLFSSH_USERAUTH_SUCCESS) {
-            /* if this is a certificate and no specific authorized keys file has
-             * been set then rely on CA to have verified the cert */
-            if (authData->sf.publicKey.isCert &&
-                    !wolfSSHD_ConfigGetAuthKeysFileSet(authCtx->conf)) {
-                wolfSSH_Log(WS_LOG_INFO,
-                    "[SSHD] Relying on CA for public key check");
-            #ifdef WIN32
-                /* Still need to get users token on Windows */
-                rc = SetupUserTokenWin(usr, &authData->sf.publicKey,
-                    wolfSSHD_ConfigGetUserCAKeysFile(authCtx->conf), authCtx);
-                if (rc == WSSHD_AUTH_SUCCESS) {
-                    wolfSSH_Log(WS_LOG_INFO, "[SSHD] Got users token ok.");
-                    ret = WOLFSSH_USERAUTH_SUCCESS;
-                }
-                else {
-                    wolfSSH_Log(WS_LOG_ERROR,
-                        "[SSHD] Error getting users token.");
-                    ret = WOLFSSH_USERAUTH_FAILURE;
-                }
-            #else
+        /* if this is a certificate and no specific authorized keys file has
+            * been set then rely on CA to have verified the cert */
+        if (authData->sf.publicKey.isCert &&
+                !wolfSSHD_ConfigGetAuthKeysFileSet(authCtx->conf)) {
+            wolfSSH_Log(WS_LOG_INFO,
+                "[SSHD] Relying on CA for public key check");
+        #ifdef WIN32
+            /* Still need to get users token on Windows */
+            rc = SetupUserTokenWin(usr, &authData->sf.publicKey,
+                wolfSSHD_ConfigGetUserCAKeysFile(authCtx->conf), authCtx);
+            if (rc == WSSHD_AUTH_SUCCESS) {
+                wolfSSH_Log(WS_LOG_INFO, "[SSHD] Got users token ok.");
                 ret = WOLFSSH_USERAUTH_SUCCESS;
-            #endif
             }
             else {
-                /* if not a certificate then parse through authorized key file */
-                rc = authCtx->checkPublicKeyCb(usr, &authData->sf.publicKey,
-                                wolfSSHD_ConfigGetUserCAKeysFile(authCtx->conf),
-                                authCtx);
-                if (rc == WSSHD_AUTH_SUCCESS) {
-                    wolfSSH_Log(WS_LOG_INFO, "[SSHD] Public key ok.");
-                    ret = WOLFSSH_USERAUTH_SUCCESS;
-                }
-                else if (rc == WSSHD_AUTH_FAILURE) {
-                    wolfSSH_Log(WS_LOG_INFO,
-                        "[SSHD] Public key not authorized.");
-                    ret = WOLFSSH_USERAUTH_INVALID_PUBLICKEY;
-                }
-                else {
-                    wolfSSH_Log(WS_LOG_ERROR,
-                        "[SSHD] Error checking public key.");
-                    ret = WOLFSSH_USERAUTH_FAILURE;
-                }
+                wolfSSH_Log(WS_LOG_ERROR,
+                    "[SSHD] Error getting users token.");
+                ret = WOLFSSH_USERAUTH_FAILURE;
+            }
+        #else
+            ret = WOLFSSH_USERAUTH_SUCCESS;
+        #endif
+        }
+        else {
+            /* if not a certificate then parse through authorized key file */
+            rc = authCtx->checkPublicKeyCb(usr, &authData->sf.publicKey,
+                            wolfSSHD_ConfigGetUserCAKeysFile(authCtx->conf),
+                            authCtx);
+            if (rc == WSSHD_AUTH_SUCCESS) {
+                wolfSSH_Log(WS_LOG_INFO, "[SSHD] Public key ok.");
+                ret = WOLFSSH_USERAUTH_SUCCESS;
+            }
+            else if (rc == WSSHD_AUTH_FAILURE) {
+                wolfSSH_Log(WS_LOG_INFO,
+                    "[SSHD] Public key not authorized.");
+                ret = WOLFSSH_USERAUTH_INVALID_PUBLICKEY;
+            }
+            else {
+                wolfSSH_Log(WS_LOG_ERROR,
+                    "[SSHD] Error checking public key.");
+                ret = WOLFSSH_USERAUTH_FAILURE;
             }
         }
     }
@@ -1545,23 +1543,23 @@ int wolfSSHD_AuthReducePermissions(WOLFSSHD_AUTH* auth)
     byte flag = 0;
     int ret = WS_SUCCESS;
 
+    if (!auth) {
+        return WS_BAD_ARGUMENT;
+    }
+
     flag = wolfSSHD_ConfigGetPrivilegeSeparation(auth->conf);
 #ifndef WIN32
     if (flag == WOLFSSHD_PRIV_SEPARAT || flag == WOLFSSHD_PRIV_SANDBOX) {
         wolfSSH_Log(WS_LOG_INFO, "[SSHD] Lowering permissions level");
-        if (auth) {
-            if (setegid(auth->gid) != 0) {
-                wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error setting sshd gid");
-                ret = WS_FATAL_ERROR;
-            }
 
-            if (seteuid(auth->uid) != 0) {
-                wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error setting sshd uid");
-                ret = WS_FATAL_ERROR;
-            }
+        if (setegid(auth->gid) != 0) {
+            wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error setting sshd gid");
+            ret = WS_FATAL_ERROR;
         }
-        else {
-            ret = WS_BAD_ARGUMENT;
+
+        if (seteuid(auth->uid) != 0) {
+            wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error setting sshd uid");
+            ret = WS_FATAL_ERROR;
         }
     }
 #endif

diff --git a/examples/portfwd/portfwd.c b/examples/portfwd/portfwd.c
@@ -337,7 +337,7 @@ THREAD_RETURN WOLFSSH_THREAD portfwd_worker(void* args)
            " * password: %s\n"
            " * forward from: %s:%u\n"
            " * forward to: %s:%u\n",
-           host, port, username, password,
+           host, port, username, password ? password : "",
            fwdFromHost, fwdFromPort,
            fwdToHost, fwdToPort);
 

diff --git a/ide/Renesas/cs+/demo_server/wolfssh_demo.c b/ide/Renesas/cs+/demo_server/wolfssh_demo.c
@@ -125,11 +125,11 @@ static int dump_stats(thread_ctx_t* ctx)
 
     wolfSSH_GetStats(ctx->ssh, &txCount, &rxCount, &seq, &peerSeq);
 
-    printf(stats, 
+    sprintf(stats, 
             "Statistics for Thread #%u:\r\n"
             "  txCount = %u\r\n  rxCount = %u\r\n"
             "  seq = %u\r\n  peerSeq = %u\r\n",
-            0, txCount, rxCount, seq, peerSeq);
+            (word32)0, txCount, rxCount, seq, peerSeq);
     statsSz = (word32)strlen(stats);
 
     fprintf(stderr, "%s", stats);
@@ -648,4 +648,4 @@ void abort(void)
 {
 
 }
-#endif
+#endif
diff --git a/src/agent.c b/src/agent.c
@@ -1303,7 +1303,10 @@ static int DoUnimplemented(WOLFSSH_AGENT_CTX* agent,
         ret = WS_BAD_ARGUMENT;
 
     WOLFSSH_UNUSED(buf);
-    DUMP(buf + *idx, len);
+
+    if (ret == WS_SUCCESS) {
+        DUMP(buf + *idx, len);
+    }
 
     /* Just skip the message. */
     if (ret == WS_SUCCESS) {
@@ -1498,11 +1501,12 @@ WOLFSSH_AGENT_CTX* wolfSSH_AGENT_new(void* heap)
 
 void wolfSSH_AGENT_free(WOLFSSH_AGENT_CTX* agent)
 {
-    void* heap = agent->heap;
+    void* heap = NULL;
 
     WLOG(WS_LOG_AGENT, "Entering wolfSSH_AGENT_free()");
 
     if (agent != NULL) {
+        heap = agent->heap;
         if (agent->msg != NULL)
             WFREE(agent->msg, agent->heap, DYNTYPE_AGENT_BUFFER);
         wc_FreeRng(&agent->rng);

diff --git a/src/internal.c b/src/internal.c
@@ -1329,7 +1329,6 @@ int IdentifyAsn1Key(const byte* in, word32 inSz, int isPrivate, void* heap)
         }
 #endif /* WOLFSSH_NO_ECDSA */
 #if !defined(WOLFSSH_NO_ED25519)
-    if (key != NULL) {
         if (key->keySigId == ID_UNKNOWN) {
             idx = 0;
             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)
 
             wc_ed25519_free(&key->ks.ed25519.key);
         }
-    }
 #endif /* WOLFSSH_NO_ED25519 */
 
         if (key->keySigId == ID_UNKNOWN) {
@@ -1623,8 +1621,7 @@ static int GetOpenSshKey(WS_KeySignature *key,
             byte keyId;
 
             idx = 0;
-            if (ret == WS_SUCCESS)
-                ret = GetUint32(&check1, str, strSz, &subIdx); /* checkint 1 */
+            ret = GetUint32(&check1, str, strSz, &subIdx); /* checkint 1 */
             if (ret == WS_SUCCESS)
                 ret = GetUint32(&check2, str, strSz, &subIdx); /* checkint 2 */
             if (ret == WS_SUCCESS) {
@@ -2815,7 +2812,7 @@ int ChannelAppend(WOLFSSH* ssh, WOLFSSH_CHANNEL* channel)
 int ChannelRemove(WOLFSSH* ssh, word32 channel, byte peer)
 {
     int ret = WS_SUCCESS;
-    WOLFSSH_CHANNEL* list;
+    WOLFSSH_CHANNEL* list = NULL;
 
     WLOG(WS_LOG_DEBUG, "Entering ChannelRemove()");
 
@@ -6380,9 +6377,7 @@ static int DoUserAuthRequestRsa(WOLFSSH* ssh, WS_UserAuthData_PublicKey* pk,
         const byte* e = NULL;
         word32 eSz = 0;
 
-        if (ret == WS_SUCCESS) {
-            ret = GetMpint(&eSz, &e, pk->publicKey, pk->publicKeySz, &i);
-        }
+        ret = GetMpint(&eSz, &e, pk->publicKey, pk->publicKeySz, &i);
 
         if (ret == WS_SUCCESS) {
             ret = GetMpint(&nSz, &n, pk->publicKey, pk->publicKeySz, &i);
@@ -13157,13 +13152,7 @@ static int BuildUserAuthRequestEcc(WOLFSSH* ssh,
     byte* checkData = NULL;
     word32 checkDataSz = 0;
 
-#ifdef WOLFSSH_SMALL_STACK
-    r_ptr = (byte*)WMALLOC(rSz, ssh->ctx->heap, DYNTYPE_BUFFER);
-    s_ptr = (byte*)WMALLOC(sSz, ssh->ctx->heap, DYNTYPE_BUFFER);
-    sig_ptr = (byte*)WMALLOC(sigSz, ssh->ctx->heap, DYNTYPE_BUFFER);
-    if (r_ptr == NULL || s_ptr == NULL || sig_ptr == NULL)
-        ret = WS_MEMORY_E;
-#else
+#ifndef WOLFSSH_SMALL_STACK
     byte r_s[ECC_MAX_SIG_SIZE / 2];
     byte s_s[ECC_MAX_SIG_SIZE / 2];
     byte sig_s[ECC_MAX_SIG_SIZE];
@@ -13178,6 +13167,14 @@ static int BuildUserAuthRequestEcc(WOLFSSH* ssh,
         return ret;
     }
 
+#ifdef WOLFSSH_SMALL_STACK
+    r_ptr = (byte*)WMALLOC(rSz, ssh->ctx->heap, DYNTYPE_BUFFER);
+    s_ptr = (byte*)WMALLOC(sSz, ssh->ctx->heap, DYNTYPE_BUFFER);
+    sig_ptr = (byte*)WMALLOC(sigSz, ssh->ctx->heap, DYNTYPE_BUFFER);
+    if (r_ptr == NULL || s_ptr == NULL || sig_ptr == NULL)
+        ret = WS_MEMORY_E;
+#endif
+
     begin = *idx;
 
     if (ret == WS_SUCCESS) {

diff --git a/src/port.c b/src/port.c
@@ -181,7 +181,7 @@ static const char* TrimFileName(const char* f, size_t* fSz)
 {
     if (f != NULL && fSz != NULL && *fSz >= 3 && f[0] == '/' && f[2] == ':') {
         f++;
-        *fSz--;
+        (*fSz)--;
     }
     return f;
 }
@@ -224,7 +224,7 @@ void* WS_CreateFileA(const char* fileName, unsigned long desiredAccess,
 void* WS_FindFirstFileA(const char* fileName,
         char* realFileName, size_t realFileNameSz, int* isDir, void* heap)
 {
-    HANDLE findHandle;
+    HANDLE findHandle = NULL;
     WIN32_FIND_DATAW findFileData;
     wchar_t* unicodeFileName;
     size_t unicodeFileNameSz = 0;
@@ -269,7 +269,7 @@ int WS_FindNextFileA(void* findHandle,
 {
     BOOL success;
     WIN32_FIND_DATAW findFileData;
-    errno_t error;
+    errno_t error = 0;
 
     success = FindNextFileW((HANDLE)findHandle, &findFileData);
 

diff --git a/src/ssh.c b/src/ssh.c
@@ -2755,15 +2755,19 @@ int wolfSSH_ChannelSend(WOLFSSH_CHANNEL* channel,
 {
     int bytesTxd = 0;
 
+    if (channel == NULL || buf == NULL) {
+        WLOG(WS_LOG_DEBUG, "Entering wolfSSH_ChannelSend() with bad argument");
+        return WS_BAD_ARGUMENT;
+    }
+
     WLOG(WS_LOG_DEBUG, "Entering wolfSSH_ChannelSend(), ID = %d, peerID = %d",
             channel->channel, channel->peerChannel);
 
 #ifdef DEBUG_WOLFSSH
     DumpOctetString(buf, bufSz);
 #endif
-    if (channel == NULL || buf == NULL)
-        bytesTxd = WS_BAD_ARGUMENT;
-    else if (!channel->openConfirmed) {
+
+    if (!channel->openConfirmed) {
         WLOG(WS_LOG_DEBUG, "Channel not confirmed yet.");
         bytesTxd = WS_CHANNEL_NOT_CONF;
     }

diff --git a/src/wolfsftp.c b/src/wolfsftp.c
@@ -3647,7 +3647,7 @@ int wolfSSH_SFTP_RecvRead(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
     word32 ofst[2] = {0, 0};
 
     byte*  out;
-    word32 outSz;
+    word32 outSz = 0;
 
     char* res  = NULL;
     char err[] = "Read File Error";
@@ -3747,7 +3747,7 @@ int wolfSSH_SFTP_RecvRead(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
     word32 idx  = 0;
 
     byte*  out;
-    word32 outSz;
+    word32 outSz = 0;
 
     char* res  = NULL;
     char err[] = "Read File Error";