Static Analysis Fixes

apps/wolfssh/common.c

@@ -31,6 +31,7 @@
 #include <wolfssh/port.h>
 #include <wolfssl/wolfcrypt/ecc.h>
 #include <wolfssl/wolfcrypt/coding.h>
+#include <wolfssl/wolfcrypt/memory.h>
 #include "apps/wolfssh/common.h"
 #ifndef USE_WINDOWS_API
     #include <termios.h>
@@ -382,10 +383,8 @@ int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx)
                             printf("This key matches other servers:\n");
                             otherMatch = 1;
                         }
-                        if (otherMatch) {
-                            printf("\t%s:%u: %s\n",
-                                    knownHostsName, lineCount, name);
-                        }
+                        printf("\t%s:%u: %s\n",
+                                knownHostsName, lineCount, name);
                     }
                 }
             }
@@ -789,9 +788,13 @@ void ClientFreeBuffers(void)
     }
 
     if (userPrivateKeyAlloc && userPrivateKey != NULL) {
+        wc_ForceZero(userPrivateKey, userPrivateKeySz);
         WFREE(userPrivateKey, NULL, DYNTYPE_PRIVKEY);
         userPrivateKey = userPrivateKeyBuf;
-        userPrivateKeySz = sizeof(userPrivateKeyBuf);
         userPrivateKeyAlloc = 0;
     }
+
+    wc_ForceZero(userPrivateKeyBuf, sizeof(userPrivateKeyBuf));
+    userPrivateKeySz = 0;
+    wc_ForceZero(userPassword, sizeof(userPassword));
 }

examples/client/common.c

@@ -33,6 +33,7 @@
 #include <wolfssl/wolfcrypt/ecc.h>
 #include <wolfssl/wolfcrypt/error-crypt.h>
 #include <wolfssl/wolfcrypt/coding.h>
+#include <wolfssl/wolfcrypt/memory.h>
 
 #ifdef WOLFSSH_TPM
     #include <wolftpm/tpm2_wrap.h>
@@ -1123,18 +1124,41 @@ void ClientFreeBuffers(const char* pubKeyName, const char* privKeyName,
     if (pubKeyName != NULL && userPublicKey != NULL &&
         userPublicKey != userPublicKeyBuf) {
         WFREE(userPublicKey, heap, DYNTYPE_PRIVKEY);
+        userPublicKey = userPublicKeyBuf;
+        userPublicKeySz = 0;
     }
 
-    if (privKeyName != NULL && userPrivateKey != NULL &&
-        userPrivateKeyAlloc) {
-        WFREE(userPrivateKey, heap, DYNTYPE_PRIVKEY);
+    if (privKeyName != NULL && userPrivateKey != NULL) {
+        wc_ForceZero(userPrivateKey, userPrivateKeySz);
+        if (userPrivateKeyAlloc) {
+            WFREE(userPrivateKey, heap, DYNTYPE_PRIVKEY);
+            userPrivateKey = userPrivateKeyBuf;
+            userPrivateKeyAlloc = 0;
+        }
     }
 
 #ifdef WOLFSSH_KEYBOARD_INTERACTIVE
-    for (entry = 0; entry < keyboardResponseCount; entry++) {
-        WFREE(keyboardResponses[entry], NULL, 0);
+    if (keyboardResponses != NULL && keyboardResponseLengths != NULL) {
+        for (entry = 0; entry < keyboardResponseCount; entry++) {
+            if (keyboardResponses[entry] != NULL &&
+                    keyboardResponseLengths[entry] != 0) {
+                wc_ForceZero(keyboardResponses[entry],
+                        keyboardResponseLengths[entry]);
+            }
+            WFREE(keyboardResponses[entry], NULL, 0);
+        }
+    }
+    if (keyboardResponses != NULL) {
+        WFREE(keyboardResponses, NULL, 0);
+        keyboardResponses = NULL;
+    }
+    if (keyboardResponseLengths != NULL) {
+        WFREE(keyboardResponseLengths, NULL, 0);
+        keyboardResponseLengths = NULL;
     }
-    WFREE(keyboardResponses, NULL, 0);
-    WFREE(keyboardResponseLengths, NULL, 0);
+    keyboardResponseCount = 0;
 #endif
+    wc_ForceZero(userPrivateKeyBuf, sizeof(userPrivateKeyBuf));
+    userPrivateKeySz = 0;
+    wc_ForceZero(userPassword, sizeof(userPassword));
 }

src/certman.c

@@ -216,7 +216,7 @@ int wolfSSH_CERTMAN_VerifyCerts_buffer(WOLFSSH_CERTMAN* cm,
 
     WLOG_ENTER();
 
-    if (cm == NULL || certs == NULL) {
+    if (cm == NULL || certs == NULL || certsCount == 0) {
         return WS_BAD_ARGUMENT;
     }
 

src/internal.c

@@ -4457,7 +4457,7 @@ static int DoKexInit(WOLFSSH* ssh, byte* buf, word32 len, word32* idx)
         algoId = MatchIdLists(side, list, listSz, cannedList, cannedListSz);
         if (algoId == ID_UNKNOWN) {
             WLOG(WS_LOG_DEBUG, "Unable to negotiate Server Host Key Algo");
-            return WS_MATCH_KEY_ALGO_E;
+            ret = WS_MATCH_KEY_ALGO_E;
         }
         else {
             ssh->handshake->pubKeyId = algoId;
@@ -4621,12 +4621,14 @@ static int DoKexInit(WOLFSSH* ssh, byte* buf, word32 len, word32* idx)
         }
     }
 
-    /* Skip the "for future use" length. */
+    /* RFC 4253 7.1 reserved field: fixed uint32 0, not a length prefix. */
     if (ret == WS_SUCCESS) {
-        WLOG(WS_LOG_DEBUG, "DKI: For Future Use");
+        WLOG(WS_LOG_DEBUG, "DKI: Reserved");
         ret = GetUint32(&skipSz, buf, len, &begin);
-        if (ret == WS_SUCCESS)
-            begin += skipSz;
+        if (ret == WS_SUCCESS && skipSz != 0) {
+            WLOG(WS_LOG_DEBUG, "DKI: non-zero reserved field");
+            ret = WS_PARSE_E;
+        }
     }
 
     if (ret == WS_SUCCESS) {
@@ -6307,10 +6309,10 @@ static int DoNewKeys(WOLFSSH* ssh, byte* buf, word32 len, word32* idx)
     int ret = WS_SUCCESS;
 
     WOLFSSH_UNUSED(buf);
-    WOLFSSH_UNUSED(len);
     WOLFSSH_UNUSED(idx);
 
-    if (ssh == NULL || ssh->handshake == NULL)
+    /* RFC 4253 7.3: SSH_MSG_NEWKEYS has no payload. */
+    if (ssh == NULL || ssh->handshake == NULL || len != 0)
         ret = WS_BAD_ARGUMENT;
 
     if (ret == WS_SUCCESS) {
@@ -10874,7 +10876,7 @@ int DoProtoId(WOLFSSH* ssh)
         }
 
         if (ssh->inputBuffer.length >= 4
-            && WSTRNCASECMP((char*)ssh->inputBuffer.buffer, "SSH-", 4) == 0)
+            && WSTRNCMP((char*)ssh->inputBuffer.buffer, "SSH-", 4) == 0)
             break;
 
         if (!allowBanner) {
@@ -10903,8 +10905,8 @@ int DoProtoId(WOLFSSH* ssh)
     eolSz = (*eol == '\r') ? 2 : 1;
 
     if (ssh->inputBuffer.length >= SSH_PROTO_SZ
-        && WSTRNCASECMP((char*)ssh->inputBuffer.buffer,
-                        ssh->ctx->sshProtoIdStr, SSH_PROTO_SZ) == 0) {
+        && WSTRNCMP((char*)ssh->inputBuffer.buffer,
+                    ssh->ctx->sshProtoIdStr, SSH_PROTO_SZ) == 0) {
 
         if (ssh->ctx->side == WOLFSSH_ENDPOINT_SERVER)
             ssh->clientState = CLIENT_VERSION_DONE;
@@ -18072,10 +18074,9 @@ int wolfSSH_TestDoKexInit(WOLFSSH* ssh, byte* buf, word32 len, word32* idx)
     return DoKexInit(ssh, buf, len, idx);
 }
 
-int wolfSSH_TestDoNewKeys(WOLFSSH* ssh)
+int wolfSSH_TestDoNewKeys(WOLFSSH* ssh, byte* buf, word32 len, word32* idx)
 {
-    /* DoNewKeys ignores buf/len/idx (marked WOLFSSH_UNUSED internally). */
-    return DoNewKeys(ssh, NULL, 0, NULL);
+    return DoNewKeys(ssh, buf, len, idx);
 }
 
 void wolfSSH_TestFreeHandshake(WOLFSSH* ssh)

src/wolfterm.c

@@ -424,11 +424,21 @@ static int wolfSSH_DoControlSeq(WOLFSSH* ssh, WOLFSSH_HANDLE handle, byte* buf,
     if (ssh->escState == WS_ESC_CSI || ssh->escBufSz > 0) {
         /* check for left overs */
         if (ssh->escBufSz > 0) {
-            word32 tmpSz = min(bufSz - *idx,
-                    WOLFSSH_MAX_CONSOLE_ARGS - (word32)ssh->escBufSz);
+            word32 tmpSz;
+
+            if (ssh->escBufSz >= WOLFSSL_MAX_ESCBUF) {
+                WLOG(WS_LOG_ERROR, "escBuf state exceeds capacity");
+                return WS_FATAL_ERROR;
+            }
+            tmpSz = min(bufSz - *idx,
+                    (word32)WOLFSSL_MAX_ESCBUF - (word32)ssh->escBufSz);
             WMEMCPY(ssh->escBuf + ssh->escBufSz, buf + *idx, tmpSz);
             i = 0;
             numArgs = getArgs(ssh->escBuf, ssh->escBufSz + tmpSz, &i, args);
+            if (i >= ssh->escBufSz + tmpSz) {
+                ssh->escBufSz += tmpSz;
+                return WS_WANT_READ;
+            }
             c = ssh->escBuf[i++];
             if (!isCommand(c)) {
                 /* expecting a command when in CSI state */

tests/api.c

@@ -645,6 +645,8 @@ static void test_wolfSSH_CTX_UsePrivateKey_buffer_pem(void)
 static void test_wolfSSH_CertMan(void)
 {
 #ifdef WOLFSSH_CERTMAN
+    /* This chunk of test is checking the innards of the WOLFSSH_CERTMAN
+     * struct which has a private declaration at the moment. */
     {
         WOLFSSH_CERTMAN* cm = NULL;
 
@@ -668,7 +670,29 @@ static void test_wolfSSH_CertMan(void)
         AssertNotNull(cmRef->heap);
         AssertEQ(cmRef->heap, fakeHeap);
     }
-#endif
+#endif /* WOLFSSH_CERTMAN */
+
+#ifdef WOLFSSH_CERTS
+    {
+        /* VerifyCerts_buffer must reject certsCount == 0; otherwise the
+         * inner loops short-circuit and the function returns WS_SUCCESS
+         * without verifying anything. */
+        WOLFSSH_CERTMAN* cm;
+        unsigned char dummy[1] = { 0 };
+
+        cm = wolfSSH_CERTMAN_new(NULL);
+        AssertNotNull(cm);
+
+        AssertIntEQ(WS_BAD_ARGUMENT,
+                wolfSSH_CERTMAN_VerifyCerts_buffer(cm, dummy, sizeof(dummy), 0));
+        AssertIntEQ(WS_BAD_ARGUMENT,
+                wolfSSH_CERTMAN_VerifyCerts_buffer(NULL, dummy, sizeof(dummy), 1));
+        AssertIntEQ(WS_BAD_ARGUMENT,
+                wolfSSH_CERTMAN_VerifyCerts_buffer(cm, NULL, 0, 1));
+
+        wolfSSH_CERTMAN_free(cm);
+    }
+#endif /* WOLFSSH_CERTS */
 }
 
 

tests/regress.c

@@ -2167,6 +2167,43 @@ static void TestFirstPacketFollows(void)
     TestFirstPacketFollowsSkipped();
 }
 
+/* RFC 4253 7.1: the trailing uint32 in KEXINIT is reserved and must be zero.
+ * DoKexInit used to advance begin by that value (treating it as a length);
+ * the current code rejects any non-zero value with WS_PARSE_E. Lock the
+ * strict-rejection branch in so a regression that re-relaxes the check or
+ * reverts to skipping skipSz bytes would fail this test. */
+static void TestKexInitReservedNonZeroRejected(void)
+{
+    WOLFSSH_CTX* ctx;
+    WOLFSSH* ssh;
+    byte payload[512];
+    word32 payloadSz;
+    word32 idx = 0;
+
+    ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL);
+    AssertNotNull(ctx);
+
+    ssh = wolfSSH_new(ctx);
+    AssertNotNull(ssh);
+    AssertIntEQ(wolfSSH_SetAlgoListKex(ssh, FPF_KEX_GOOD), WS_SUCCESS);
+    AssertIntEQ(wolfSSH_SetAlgoListKey(ssh, FPF_KEY_GOOD), WS_SUCCESS);
+
+    payloadSz = BuildKexInitPayload(ssh, FPF_KEX_GOOD, FPF_KEY_GOOD,
+            0, payload, (word32)sizeof(payload));
+
+    /* BuildKexInitPayload puts the reserved uint32 in the final 4 bytes.
+     * Overwrite them with a non-zero value to exercise the strict branch. */
+    AssertTrue(payloadSz >= UINT32_SZ);
+    (void)AppendUint32(payload, (word32)sizeof(payload),
+            payloadSz - UINT32_SZ, 0xDEADBEEFu);
+
+    AssertIntEQ(wolfSSH_TestDoKexInit(ssh, payload, payloadSz, &idx),
+            WS_PARSE_E);
+
+    wolfSSH_free(ssh);
+    wolfSSH_CTX_free(ctx);
+}
+
 #if !defined(WOLFSSH_NO_AES_CBC) && !defined(WOLFSSH_NO_AES_CTR) \
     && !defined(WOLFSSH_NO_HMAC_SHA1) && !defined(WOLFSSH_NO_HMAC_SHA2_256)
 static void TestIndependentAlgoNegotiation(void)
@@ -2963,7 +3000,7 @@ static void TestDoNewKeys(void)
 
     /* Peer has sent NewKeys; self has already sent its own (not keying). */
     ssh->isKeying = WOLFSSH_PEER_IS_KEYING;
-    AssertIntEQ(wolfSSH_TestDoNewKeys(ssh), WS_SUCCESS);
+    AssertIntEQ(wolfSSH_TestDoNewKeys(ssh, NULL, 0, NULL), WS_SUCCESS);
 
     /* handshake freed by DoNewKeys. */
     AssertTrue(ssh->handshake == NULL);
@@ -3018,7 +3055,7 @@ static void TestDoNewKeys(void)
     WMEMCPY(&savedPeerKeys, &ssh->handshake->peerKeys, sizeof(Keys));
 
     ssh->isKeying = WOLFSSH_PEER_IS_KEYING;
-    AssertIntEQ(wolfSSH_TestDoNewKeys(ssh), WS_SUCCESS);
+    AssertIntEQ(wolfSSH_TestDoNewKeys(ssh, NULL, 0, NULL), WS_SUCCESS);
 
     AssertTrue(ssh->handshake == NULL);
 
@@ -3072,7 +3109,7 @@ static void TestDoNewKeys(void)
     WMEMCPY(&savedPeerKeys, &ssh->handshake->peerKeys, sizeof(Keys));
 
     ssh->isKeying = WOLFSSH_PEER_IS_KEYING;
-    AssertIntEQ(wolfSSH_TestDoNewKeys(ssh), WS_SUCCESS);
+    AssertIntEQ(wolfSSH_TestDoNewKeys(ssh, NULL, 0, NULL), WS_SUCCESS);
 
     AssertTrue(ssh->handshake == NULL);
 
@@ -3126,7 +3163,11 @@ static void TestDoNewKeys(void)
     WMEMCPY(&savedPeerKeys, &ssh->handshake->peerKeys, sizeof(Keys));
 
     ssh->isKeying = WOLFSSH_PEER_IS_KEYING;
-    AssertIntEQ(wolfSSH_TestDoNewKeys(ssh), WS_SUCCESS);
+    /* Exercise the len != 0 rejection while handshake is still allocated,
+     * so the guard is reached and not short-circuited by handshake == NULL. */
+    AssertIntEQ(wolfSSH_TestDoNewKeys(ssh, NULL, 1, NULL), WS_BAD_ARGUMENT);
+    AssertNotNull(ssh->handshake);
+    AssertIntEQ(wolfSSH_TestDoNewKeys(ssh, NULL, 0, NULL), WS_SUCCESS);
 
     AssertTrue(ssh->handshake == NULL);
 
@@ -3190,6 +3231,7 @@ int main(int argc, char** argv)
     && !defined(WOLFSSH_NO_CURVE25519_SHA256) \
     && !defined(WOLFSSH_NO_RSA_SHA2_256)
     TestFirstPacketFollows();
+    TestKexInitReservedNonZeroRejected();
 #endif
 #if !defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) && !defined(WOLFSSH_NO_RSA) \
     && !defined(WOLFSSH_NO_CURVE25519_SHA256) \

tests/unit.c

@@ -87,13 +87,21 @@ static const ProtoIdTestVector protoIdTestVectors[] = {
       "SSH-2.0-wolfSSHv" LIBWOLFSSH_VERSION_STRING "\n",
       0, WS_SUCCESS, WOLFSSH_ENDPOINT_CLIENT },
 
-    /* Case-insensitive match. DoProtoId uses WSTRNCASECMP. */
+    /* Case rejection. DoProtoId uses WSTRNCMP. */
     { "lowercase ssh prefix",
       "ssh-2.0-wolfSSHv" LIBWOLFSSH_VERSION_STRING "\r\n",
-      0, WS_SUCCESS, WOLFSSH_ENDPOINT_CLIENT },
+      0, WS_SOCKET_ERROR_E, WOLFSSH_ENDPOINT_CLIENT },
     { "mixed case SSH prefix",
       "Ssh-2.0-wolfSSHv" LIBWOLFSSH_VERSION_STRING "\r\n",
-      0, WS_SUCCESS, WOLFSSH_ENDPOINT_CLIENT },
+      0, WS_SOCKET_ERROR_E, WOLFSSH_ENDPOINT_CLIENT },
+
+    /* Case rejection. DoProtoId uses WSTRNCMP. */
+    { "lowercase ssh prefix",
+      "ssh-2.0-wolfSSHv" LIBWOLFSSH_VERSION_STRING "\r\n",
+      0, WS_VERSION_E, WOLFSSH_ENDPOINT_SERVER },
+    { "mixed case SSH prefix",
+      "Ssh-2.0-wolfSSHv" LIBWOLFSSH_VERSION_STRING "\r\n",
+      0, WS_VERSION_E, WOLFSSH_ENDPOINT_SERVER },
 
     /* OpenSSH peer identification. */
     { "OpenSSH version string",

wolfssh/internal.h

@@ -1358,8 +1358,9 @@ enum WS_MessageIdLimits {
             word32 len, word32* idx);
     WOLFSSH_API int wolfSSH_TestDoKexInit(WOLFSSH* ssh, byte* buf,
             word32 len, word32* idx);
+    WOLFSSH_API int wolfSSH_TestDoNewKeys(WOLFSSH* ssh, byte* buf,
+            word32 len, word32* idx);
     WOLFSSH_API int wolfSSH_TestGenerateKeys(WOLFSSH* ssh, byte hashId);
-    WOLFSSH_API int wolfSSH_TestDoNewKeys(WOLFSSH* ssh);
     WOLFSSH_API void wolfSSH_TestFreeHandshake(WOLFSSH* ssh);
     WOLFSSH_API int wolfSSH_TestDoKexDhInit(WOLFSSH* ssh, byte* buf,
             word32 len, word32* idx);