Fix multiple reported issues

src/agent.c

@@ -61,6 +61,11 @@
 #endif
 
 
+#ifndef WOLFSSH_MAX_PASSPHRASE_SZ
+    #define WOLFSSH_MAX_PASSPHRASE_SZ 256
+#endif
+
+
 /* payloadSz is an estimate, but it shall be greater-than/equal-to
  * the actual value. */
 static int PrepareMessage(WOLFSSH_AGENT_CTX* agent, word32 payloadSz)
@@ -370,45 +375,57 @@ static int PostSuccess(WOLFSSH_AGENT_CTX* agent)
 static int PostLock(WOLFSSH_AGENT_CTX* agent,
         const byte* passphrase, word32 passphraseSz)
 {
-    char pp[32];
-    word32 ppSz;
+    char* pp;
+    int ret = WS_SUCCESS;
 
-    (void)agent;
     WLOG(WS_LOG_AGENT, "Posting lock to agent %p", agent);
-    WOLFSSH_UNUSED(agent);
 
-    ppSz = sizeof(pp) - 1;
-    if (passphraseSz < ppSz)
-        ppSz = passphraseSz;
+    pp = (char*)WMALLOC(passphraseSz + 1, agent->heap, DYNTYPE_STRING);
+    if (pp == NULL)
+        ret = WS_MEMORY_E;
 
-    WMEMCPY(pp, passphrase, ppSz);
-    pp[ppSz] = 0;
-    WLOG(WS_LOG_AGENT, "Locking with passphrase '%s'", pp);
+    if (ret == WS_SUCCESS) {
+        WMEMCPY(pp, passphrase, passphraseSz);
+        pp[passphraseSz] = 0;
+#ifdef SHOW_SECRETS
+        WLOG(WS_LOG_AGENT, "Locking with passphrase '%s'", pp);
+#else
+        WLOG(WS_LOG_AGENT, "Locking with passphrase");
+#endif
+        ForceZero(pp, passphraseSz);
+        WFREE(pp, agent->heap, DYNTYPE_STRING);
+    }
 
-    return WS_SUCCESS;
+    return ret;
 }
 
 
 /* Stub that has not yet been implemented */
 static int PostUnlock(WOLFSSH_AGENT_CTX* agent,
         const byte* passphrase, word32 passphraseSz)
 {
-    char pp[32];
-    word32 ppSz;
+    char* pp;
+    int ret = WS_SUCCESS;
 
-    (void)agent;
     WLOG(WS_LOG_AGENT, "Posting unlock to agent %p", agent);
-    WOLFSSH_UNUSED(agent);
 
-    ppSz = sizeof(pp) - 1;
-    if (passphraseSz < ppSz)
-        ppSz = passphraseSz;
+    pp = (char*)WMALLOC(passphraseSz + 1, agent->heap, DYNTYPE_STRING);
+    if (pp == NULL)
+        ret = WS_MEMORY_E;
 
-    WMEMCPY(pp, passphrase, ppSz);
-    pp[ppSz] = 0;
-    WLOG(WS_LOG_AGENT, "Unlocking with passphrase '%s'", pp);
+    if (ret == WS_SUCCESS) {
+        WMEMCPY(pp, passphrase, passphraseSz);
+        pp[passphraseSz] = 0;
+#ifdef SHOW_SECRETS
+        WLOG(WS_LOG_AGENT, "Unlocking with passphrase '%s'", pp);
+#else
+        WLOG(WS_LOG_AGENT, "Unlocking with passphrase");
+#endif
+        ForceZero(pp, passphraseSz);
+        WFREE(pp, agent->heap, DYNTYPE_STRING);
+    }
 
-    return WS_SUCCESS;
+    return ret;
 }
 
 
@@ -672,7 +689,8 @@ static WOLFSSH_AGENT_ID* FindKeyId(WOLFSSH_AGENT_ID* id,
     if (ret == WS_SUCCESS) {
         while (id != NULL &&
                 WMEMCMP(digest, id->id, WC_SHA256_DIGEST_SIZE) != 0 &&
-                WMEMCMP(keyBlob, id->keyBlob, keyBlobSz)) {
+                (id->keyBlobSz != keyBlobSz ||
+                 WMEMCMP(keyBlob, id->keyBlob, id->keyBlobSz) != 0)) {
             id = id->next;
         }
     }
@@ -1252,7 +1270,8 @@ static int DoAgentLock(WOLFSSH_AGENT_CTX* agent,
         ato32(buf + begin, &passphraseSz);
         begin += LENGTH_SZ;
 
-        if (begin + passphraseSz > len) {
+        if (passphraseSz > WOLFSSH_MAX_PASSPHRASE_SZ ||
+                begin + passphraseSz > len) {
             ret = WS_PARSE_E;
         }
     }
@@ -1296,7 +1315,8 @@ static int DoAgentUnlock(WOLFSSH_AGENT_CTX* agent,
         ato32(buf + begin, &passphraseSz);
         begin += LENGTH_SZ;
 
-        if (begin + passphraseSz > len)
+        if (passphraseSz > WOLFSSH_MAX_PASSPHRASE_SZ ||
+                begin + passphraseSz > len)
             ret = WS_PARSE_E;
     }
 

src/certman.c

@@ -174,14 +174,16 @@ void wolfSSH_CERTMAN_free(WOLFSSH_CERTMAN* cm)
 int wolfSSH_CERTMAN_LoadRootCA_buffer(WOLFSSH_CERTMAN* cm,
         const unsigned char* rootCa, word32 rootCaSz)
 {
-    int ret;
+    int ret = WS_BAD_ARGUMENT;
 
     WLOG_ENTER();
 
-    ret = wolfSSL_CertManagerLoadCABuffer(cm->cm, rootCa, rootCaSz,
-            WOLFSSL_FILETYPE_ASN1);
-    if (ret == WOLFSSL_SUCCESS) {
-        ret = WS_SUCCESS;
+    if (cm != NULL && rootCa != NULL && rootCaSz > 0) {
+        ret = wolfSSL_CertManagerLoadCABuffer(cm->cm, rootCa, rootCaSz,
+                WOLFSSL_FILETYPE_ASN1);
+        if (ret == WOLFSSL_SUCCESS) {
+            ret = WS_SUCCESS;
+        }
     }
 
     WLOG_LEAVE(ret);

src/internal.c

@@ -5055,16 +5055,24 @@ static int DoKexInit(WOLFSSH* ssh, byte* buf, word32 len, word32* idx)
     if (ret == WS_SUCCESS) {
         WLOG(WS_LOG_DEBUG, "DKI: Languages - Client to Server");
         ret = GetUint32(&skipSz, buf, len, &begin);
-        if (ret == WS_SUCCESS)
-            begin += skipSz;
+        if (ret == WS_SUCCESS) {
+            if (skipSz <= len - begin)
+                begin += skipSz;
+            else
+                ret = WS_BUFFER_E;
+        }
     }
 
     /* Languages - Server to Client, skip */
     if (ret == WS_SUCCESS) {
         WLOG(WS_LOG_DEBUG, "DKI: Languages - Server to Client");
         ret = GetUint32(&skipSz, buf, len, &begin);
-        if (ret == WS_SUCCESS)
-            begin += skipSz;
+        if (ret == WS_SUCCESS) {
+            if (skipSz <= len - begin)
+                begin += skipSz;
+            else
+                ret = WS_BUFFER_E;
+        }
     }
 
     /* First KEX Packet Follows */

src/port.c

@@ -839,7 +839,18 @@ char* wstrnstr(const char* s1, const char* s2, unsigned int n)
  * end of s1 including a null terminator. */
 char* wstrncat(char* s1, const char* s2, size_t n)
 {
-    size_t freeSpace = n - strlen(s1) - 1;
+    size_t s1_len = 0;
+    size_t freeSpace;
+
+    while (s1_len < n && s1[s1_len] != '\0') {
+        s1_len++;
+    }
+
+    if (s1_len >= n) {
+        return NULL;
+    }
+
+    freeSpace = n - s1_len - 1;
 
     if (freeSpace >= strlen(s2)) {
         #ifndef USE_WINDOWS_API

src/ssh.c

@@ -3835,7 +3835,7 @@ int wolfSSH_RealPath(const char* defaultPath, char* in,
         }
         /* Everything else is copied */
         else {
-            if (curSz >= outSz - segSz) {
+            if (segSz > outSz || curSz >= outSz - segSz) {
                 return WS_INVALID_PATH_E;
             }
 

src/wolfscp.c

@@ -39,6 +39,9 @@
 #include <wolfssh/internal.h>
 #include <wolfssh/log.h>
 
+#include <errno.h>
+#include <stdint.h>
+
 
 #ifdef NO_INLINE
     #include <wolfssh/misc.h>
@@ -1093,19 +1096,35 @@ static int GetScpFileSize(WOLFSSH* ssh, byte* buf, word32 bufSz,
         ret = WS_SCP_BAD_MSG_E;
 
     if (ret == WS_SUCCESS) {
-        /* replace space with newline for atoi */
-        buf[spaceIdx] = '\n';
-        ssh->scpFileSz = atoi((char *)(buf + idx));
+        /* replace space with newline to terminate the size field, then parse
+         * with strtoull() which parses in 64-bit width, so a negative field
+         * such as "-1" wraps above UINT32_MAX and is rejected by the bound
+         * below instead of becoming a huge word32 size */
+        char* endptr = NULL;
+        word64 fileSz;
 
-        /* restore space, increment idx to space */
+        buf[spaceIdx] = '\n';
+        errno = 0;
+        fileSz = (word64)strtoull((char*)(buf + idx), &endptr, 10);
         buf[spaceIdx] = ' ';
-        idx = spaceIdx;
 
-        /* eat trailing space */
-        if (bufSz >= (word32)(idx + 1))
-            idx++;
+        /* reject any parse error (e.g. ERANGE overflow), a non-numeric field
+         * (parse must consume every character up to the separator), and
+         * sizes too large for the word32 scpFileSz */
+        if (errno != 0 || endptr != (char*)(buf + spaceIdx) ||
+            fileSz > UINT32_MAX) {
+            ret = WS_SCP_BAD_MSG_E;
+        }
+        else {
+            ssh->scpFileSz = (word32)fileSz;
 
-        *inOutIdx = idx;
+            /* increment idx to space, then eat trailing space */
+            idx = spaceIdx;
+            if (bufSz >= (word32)(idx + 1))
+                idx++;
+
+            *inOutIdx = idx;
+        }
     }
 
     return ret;
@@ -1225,15 +1244,23 @@ static int GetScpTimestamp(WOLFSSH* ssh, byte* buf, word32 bufSz,
 
     /* read modification time */
     if (ret == WS_SUCCESS) {
-        /* replace space with newline for atoi */
-        buf[spaceIdx] = '\n';
-        ssh->scpMTime = atoi((char*)(buf + idx));
+        char* endptr = NULL;
 
-        /* restore space, increment idx past it */
+        /* replace space with newline to terminate the field */
+        buf[spaceIdx] = '\n';
+        errno = 0;
+        ssh->scpMTime = (word64)strtoull((char*)(buf + idx), &endptr, 10);
         buf[spaceIdx] = ' ';
-        if (spaceIdx + 1 < bufSz) {
+
+        /* reject any parse error (e.g. ERANGE overflow) and a non-numeric
+         * field, then step past the separating space */
+        if (errno != 0 || endptr != (char*)(buf + spaceIdx)) {
+            ret = WS_SCP_TIMESTAMP_E;
+        }
+        else if (spaceIdx + 1 < bufSz) {
             idx = spaceIdx + 1;
-        } else {
+        }
+        else {
             ret = WS_SCP_TIMESTAMP_E;
         }
     }
@@ -1264,15 +1291,21 @@ static int GetScpTimestamp(WOLFSSH* ssh, byte* buf, word32 bufSz,
     }
 
     if (ret == WS_SUCCESS) {
-        /* replace space with newline for atoi */
+        char* endptr = NULL;
+        /* replace space with newline for strtoull */
         buf[spaceIdx] = '\n';
-        ssh->scpATime = atoi((char*)(buf + idx));
-
+        errno = 0;
+        ssh->scpATime = (word64)strtoull((char*)(buf + idx), &endptr, 10);
         /* restore space, increment idx past it */
         buf[spaceIdx] = ' ';
-        if (spaceIdx + 1 < bufSz) {
+
+        if (errno != 0 || endptr != (char*)(buf + spaceIdx)) {
+            ret = WS_SCP_TIMESTAMP_E;
+        }
+        else if (spaceIdx + 1 < bufSz) {
             idx = spaceIdx + 1;
-        } else {
+        }
+        else {
             ret = WS_SCP_TIMESTAMP_E;
         }
     }