Memory-safety, zeroization, and correctness fixes across agent, SFTP, terminal, FPKI, and transport

apps/wolfssh/common.c

@@ -392,11 +392,16 @@ int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx)
         fp[0] = 0;
 
         /* Get the key type out of the key. */
-        ato32(pubKey, &sz);
-        if ((sz > pubKeySz - sizeof(word32))
-                    || (sz > WOLFSSH_CLIENT_PUBKEYTYPE_SIZE_ESTIMATE - 1)) {
+        if (pubKeySz < sizeof(word32)) {
             ret = -1;
         }
+        else {
+            ato32(pubKey, &sz);
+            if ((sz > pubKeySz - sizeof(word32))
+                        || (sz > WOLFSSH_CLIENT_PUBKEYTYPE_SIZE_ESTIMATE - 1)) {
+                ret = -1;
+            }
+        }
     }
 
     if (ret == 0) {

apps/wolfssh/wolfssh.c

@@ -563,7 +563,7 @@ static THREAD_RET readPeer(void* in)
                 buf[bufSz - 1] = '\0';
 
             #ifdef USE_WINDOWS_API
-                if (WriteFile(stdoutHandle, buf, bufSz, &writtn, NULL) == FALSE) {
+                if (WriteFile(stdoutHandle, buf, (DWORD)ret, &writtn, NULL) == FALSE) {
                     err_sys("Failed to write to stdout handle");
                 }
             #else
@@ -857,6 +857,10 @@ static int config_parse_command_line(struct config* config,
         }
 
         command = (char*)WMALLOC(commandSz, NULL, 0);
+        if (command == NULL) {
+            fprintf(stderr, "Couldn't capture the command.\n");
+            exit(EXIT_FAILURE);
+        }
         config->command = command;
         cursor = command;
 

apps/wolfsshd/configuration.c

@@ -787,9 +787,13 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value, int depth)
                         /* Skip sub-directories */
                     #if defined(__QNX__) || defined(__QNXNTO__)
                         struct stat s;
+                        int pathLen;
 
-                        lstat(dir->d_name, &s);
-                        if (!S_ISDIR(s.st_mode))
+                        pathLen = WSNPRINTF(filepath, PATH_MAX, "%s/%s",
+                                path, dir->d_name);
+                        if (pathLen > 0 && pathLen < PATH_MAX &&
+                                lstat(filepath, &s) == 0 &&
+                                !S_ISDIR(s.st_mode))
                     #else
                         if (dir->d_type != DT_DIR)
                     #endif
@@ -819,9 +823,13 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value, int depth)
                             /* Skip sub-directories */
                         #if defined(__QNX__) || defined(__QNXNTO__)
                             struct stat s;
+                            int pathLen;
 
-                            lstat(dir->d_name, &s);
-                            if (!S_ISDIR(s.st_mode))
+                            pathLen = WSNPRINTF(filepath, PATH_MAX, "%s/%s",
+                                    path, dir->d_name);
+                            if (pathLen > 0 && pathLen < PATH_MAX &&
+                                    lstat(filepath, &s) == 0 &&
+                                    !S_ISDIR(s.st_mode))
                         #else
                             if (dir->d_type != DT_DIR)
                         #endif

examples/client/client.c

@@ -546,7 +546,7 @@ static THREAD_RET readPeer(void* in)
                 buf[bufSz - 1] = '\0';
 
             #ifdef USE_WINDOWS_API
-                if (WriteFile(stdoutHandle, buf, bufSz, &writtn, NULL) == FALSE) {
+                if (WriteFile(stdoutHandle, buf, (DWORD)ret, &writtn, NULL) == FALSE) {
                     err_sys("Failed to write to stdout handle");
                 }
             #else

examples/client/common.c

@@ -519,6 +519,8 @@ int ClientUserAuth(byte authType,
     else if (authType == WOLFSSH_USERAUTH_PASSWORD) {
         if (defaultPassword != NULL) {
             passwordSz = (word32)strlen(defaultPassword);
+            if (passwordSz > (word32)sizeof(userPassword))
+                passwordSz = (word32)sizeof(userPassword);
             memcpy(userPassword, defaultPassword, passwordSz);
         }
 #ifdef WOLFSSH_TERM

examples/echoserver/echoserver.c

@@ -299,7 +299,6 @@ static void *global_req(void *ctx)
     int ret;
     const char str[] = "SampleRequest";
     thread_ctx_t *threadCtx = (thread_ctx_t *)ctx;
-    byte buf[0];
 
     wolfSSH_SetReqSuccess(threadCtx->ctx, callbackReqSuccess);
     wolfSSH_SetReqSuccessCtx(threadCtx->ssh, &threadCtx->ssh); /* dummy ctx */
@@ -318,14 +317,6 @@ static void *global_req(void *ctx)
             wolfSSH_shutdown(threadCtx->ssh);
             return NULL;
         }
-
-        wolfSSH_stream_read(threadCtx->ssh, buf, 0);
-        if (ret != WS_SUCCESS)
-        {
-            printf("wolfSSH_stream_read Failed.\n");
-            wolfSSH_shutdown(threadCtx->ssh);
-            return NULL;
-        }
     }
     return NULL;
 }

examples/portfwd/portfwd.c

@@ -172,6 +172,8 @@ static int wsUserAuth(byte authType,
     (void)authType;
     if (defaultPassword != NULL) {
         passwordSz = (word32)strlen(defaultPassword);
+        if (passwordSz > (word32)sizeof(userPassword))
+            passwordSz = (word32)sizeof(userPassword);
         memcpy(userPassword, defaultPassword, passwordSz);
     }
     else {

src/agent.c

@@ -383,7 +383,8 @@ static int PostLock(WOLFSSH_AGENT_CTX* agent,
 
     WMEMCPY(pp, passphrase, ppSz);
     pp[ppSz] = 0;
-    WLOG(WS_LOG_AGENT, "Locking with passphrase '%s'", pp);
+    WLOG(WS_LOG_AGENT, "Locking with passphrase");
+    ForceZero(pp, sizeof(pp));
 
     return WS_SUCCESS;
 }
@@ -406,7 +407,8 @@ static int PostUnlock(WOLFSSH_AGENT_CTX* agent,
 
     WMEMCPY(pp, passphrase, ppSz);
     pp[ppSz] = 0;
-    WLOG(WS_LOG_AGENT, "Unlocking with passphrase '%s'", pp);
+    WLOG(WS_LOG_AGENT, "Unlocking with passphrase");
+    ForceZero(pp, sizeof(pp));
 
     return WS_SUCCESS;
 }
@@ -670,9 +672,11 @@ static WOLFSSH_AGENT_ID* FindKeyId(WOLFSSH_AGENT_ID* id,
     wc_Sha256Free(&sha);
 
     if (ret == WS_SUCCESS) {
+        /* only compare equal-length blobs to avoid an over-read */
         while (id != NULL &&
                 WMEMCMP(digest, id->id, WC_SHA256_DIGEST_SIZE) != 0 &&
-                WMEMCMP(keyBlob, id->keyBlob, keyBlobSz)) {
+                (keyBlobSz != id->keyBlobSz ||
+                 WMEMCMP(keyBlob, id->keyBlob, keyBlobSz) != 0)) {
             id = id->next;
         }
     }
@@ -1386,7 +1390,8 @@ static int DoMessage(WOLFSSH_AGENT_CTX* agent,
         ato32(buf + begin, &payloadSz);
         WLOG(WS_LOG_AGENT, "payloadSz = %u", payloadSz);
         begin += LENGTH_SZ;
-        if (payloadSz > len - begin) {
+        /* reject 0: payloadSz - 1 is used as a length below */
+        if (payloadSz == 0 || payloadSz > len - begin) {
             ret = WS_OVERFLOW_E;
         }
     }
@@ -1582,7 +1587,7 @@ void wolfSSH_AGENT_ID_free(WOLFSSH_AGENT_ID* id, void* heap)
 
     if (id != NULL) {
         if (id->keyBuffer != NULL) {
-            WMEMSET(id->keyBuffer, 0, id->keyBufferSz);
+            ForceZero(id->keyBuffer, id->keyBufferSz);
             WFREE(id->keyBuffer, heap, DYNTYPE_STRING);
         }
         WMEMSET(id, 0, sizeof(WOLFSSH_AGENT_ID));
@@ -1682,14 +1687,10 @@ int wolfSSH_AGENT_worker(WOLFSSH* ssh)
     if (ssh == NULL)
         ret = WS_SSH_NULL_E;
 
-    while (1) {
-        if (ret == WS_SUCCESS) {
-
-            SendSuccess(NULL);
-            DoMessage(NULL, NULL, 0, NULL);
-            ssh->agent->state = AGENT_STATE_DONE;
-            break;
-        }
+    if (ret == WS_SUCCESS) {
+        SendSuccess(NULL);
+        DoMessage(NULL, NULL, 0, NULL);
+        ssh->agent->state = AGENT_STATE_DONE;
     }
 
     WLOG_LEAVE(ret);

src/certman.c

@@ -560,6 +560,18 @@ static int CheckProfile(DecodedCert* cert, int profile)
         }
     }
 
+    if (valid) {
+        valid =
+            cert->signatureOID == CTC_SHA256wRSA ||
+            cert->signatureOID == CTC_SHA384wRSA ||
+            cert->signatureOID == CTC_SHA512wRSA ||
+            cert->signatureOID == CTC_SHA256wECDSA ||
+            cert->signatureOID == CTC_SHA384wECDSA ||
+            cert->signatureOID == CTC_SHA512wECDSA;
+        if (valid != 1)
+            WLOG(WS_LOG_CERTMAN, "cert signature algorithm not FPKI approved");
+    }
+
 #ifdef DEBUG_WOLFSSH
     switch (profile) {
         case PROFILE_FPKI_WORKSHEET_6:

src/internal.c

@@ -2794,6 +2794,9 @@ static int SetHostPrivateKey(WOLFSSH_CTX* ctx,
     }
 
     if (destIdx >= WOLFSSH_MAX_PVT_KEYS) {
+        /* der not taken on this path; free it to avoid a leak */
+        ForceZero(der, derSz);
+        WFREE(der, ctx->heap, dynamicType);
         ret = WS_CTX_KEY_COUNT_E;
     }
     else {
@@ -6666,6 +6669,8 @@ static int KeyAgreeEcdhMlKem_client(WOLFSSH* ssh, byte hashId,
 
     wc_MlKemKey_Free(&kem);
 
+    ForceZero(ssh->handshake->x, ssh->handshake->xSz);
+
     /* Replace the concatenated shared secrets with the hash. That
      * will become the new shared secret. */
     if (ret == 0) {
@@ -19705,8 +19710,9 @@ int wolfSSH_CleanPath(WOLFSSH* ssh, char* in, int inSz)
                 if (path[i] == WS_DELIM) {
                     int z;
 
-                    /* if next two chars are .. then delete */
-                    if (path[i+1] == '.' && path[i+2] == '.') {
+                    /* delete only a real ".." segment, not "..name" */
+                    if (path[i+1] == '.' && path[i+2] == '.' &&
+                            (path[i+3] == WS_DELIM || path[i+3] == '\0')) {
                         enIdx = i + 3;
 
                         /* start at one char before / and retrace path */

src/log.c

@@ -49,7 +49,7 @@
 #endif
 
 
-#ifndef WOLFSSL_NO_DEFAULT_LOGGING_CB
+#ifndef WOLFSSH_NO_DEFAULT_LOGGING_CB
     static void DefaultLoggingCb(enum wolfSSH_LogLevel level,
             const char *const msgStr);
     static wolfSSH_LoggingCb logFunction = DefaultLoggingCb;

src/ssh.c

@@ -3835,15 +3835,16 @@ int wolfSSH_RealPath(const char* defaultPath, char* in,
         }
         /* Everything else is copied */
         else {
-            if (curSz >= outSz - segSz) {
-                return WS_INVALID_PATH_E;
-            }
-
+            /* WSTRNCAT size arg is the whole buffer; NULL if seg won't fit */
             if (curSz != 1) {
-                WSTRNCAT(out, "/", outSz - curSz);
+                if (WSTRNCAT(out, "/", outSz) == NULL) {
+                    return WS_INVALID_PATH_E;
+                }
                 curSz++;
             }
-            WSTRNCAT(out, seg, outSz - curSz);
+            if (WSTRNCAT(out, seg, outSz) == NULL) {
+                return WS_INVALID_PATH_E;
+            }
             curSz += segSz;
         }
     }

src/wolfsftp.c

@@ -850,6 +850,7 @@ static void wolfSSH_SFTP_ClearState(WOLFSSH* ssh, enum WS_SFTP_STATE_ID state)
 
         if (state & STATE_ID_GET) {
             if (ssh->getState) {
+                ForceZero(ssh->getState->r, WOLFSSH_MAX_SFTP_RW);
                 WFREE(ssh->getState, ssh->ctx->heap, DYNTYPE_SFTP_STATE);
                 ssh->getState = NULL;
             }
@@ -928,6 +929,7 @@ static void wolfSSH_SFTP_ClearState(WOLFSSH* ssh, enum WS_SFTP_STATE_ID state)
 
         if (state & STATE_ID_PUT) {
             if (ssh->putState) {
+                ForceZero(ssh->putState->r, WOLFSSH_MAX_SFTP_RW);
                 WFREE(ssh->putState, ssh->ctx->heap, DYNTYPE_SFTP_STATE);
                 ssh->putState = NULL;
             }
@@ -7278,7 +7280,9 @@ static int wolfSSH_SFTP_GetHandle(WOLFSSH* ssh, byte* handle, word32* handleSz)
                  * max size */
                 wolfSSH_SFTP_buffer_rewind(&state->buffer);
                 if (wolfSSH_SFTP_buffer_ato32(&state->buffer, &sz) != WS_SUCCESS
-                       || sz > WOLFSSH_MAX_HANDLE || *handleSz < sz) {
+                       || sz > WOLFSSH_MAX_HANDLE || *handleSz < sz
+                       || UINT32_SZ + sz
+                              > wolfSSH_SFTP_buffer_size(&state->buffer)) {
                     WLOG(WS_LOG_SFTP, "Handle size found was too big");
                     WLOG(WS_LOG_SFTP, "Check size set in input handleSz");
                     ssh->error = WS_BUFFER_E;
@@ -9652,7 +9656,7 @@ int wolfSSH_SFTP_Get(WOLFSSH* ssh, char* from,
             #elif defined(USE_WINDOWS_API)
                     {
                         DWORD desiredAccess = GENERIC_WRITE;
-                        if (state->gOfst > 0)
+                        if (state->gOfst[0] > 0 || state->gOfst[1] > 0)
                             desiredAccess |= FILE_APPEND_DATA;
                         state->fileHandle = WS_CreateFileA(to, desiredAccess,
                             (FILE_SHARE_DELETE | FILE_SHARE_READ |
@@ -9775,6 +9779,7 @@ int wolfSSH_SFTP_Get(WOLFSSH* ssh, char* from,
             case STATE_GET_CLEANUP:
                 WLOG(WS_LOG_SFTP, "SFTP GET STATE: CLEANUP");
                 if (ssh->getState != NULL) {
+                    ForceZero(ssh->getState->r, WOLFSSH_MAX_SFTP_RW);
                     WFREE(ssh->getState, ssh->ctx->heap, DYNTYPE_SFTP_STATE);
                     ssh->getState = NULL;
                 }
@@ -9998,6 +10003,7 @@ int wolfSSH_SFTP_Put(WOLFSSH* ssh, char* from, char* to, byte resume,
             case STATE_PUT_CLEANUP:
                 WLOG(WS_LOG_SFTP, "SFTP PUT STATE: CLEANUP");
                 if (ssh->putState != NULL) {
+                    ForceZero(ssh->putState->r, WOLFSSH_MAX_SFTP_RW);
                     WFREE(ssh->putState, ssh->ctx->heap, DYNTYPE_SFTP_STATE);
                     ssh->putState = NULL;
                 }

src/wolfterm.c

@@ -473,6 +473,15 @@ static int wolfSSH_DoControlSeq(WOLFSSH* ssh, WOLFSSH_HANDLE handle, byte* buf,
         }
         else {
             numArgs = getArgs(buf, bufSz, &i, args);
+            if (i >= bufSz) {
+                /* save left overs for next call */
+                if (bufSz - *idx > WOLFSSL_MAX_ESCBUF) {
+                    return WS_FATAL_ERROR;
+                }
+                WMEMCPY(ssh->escBuf, buf + *idx, bufSz - *idx);
+                ssh->escBufSz = bufSz - *idx;
+                return WS_WANT_READ;
+            }
             c = buf[i]; i++;
         }
     }
@@ -669,6 +678,7 @@ int wolfSSH_ConvertConsole(WOLFSSH* ssh, WOLFSSH_HANDLE handle, byte* buf,
             if (ret == WS_WANT_READ) {
                 return ret;
             }
+            ssh->escState = WC_ESC_NONE;
             ssh->escBufSz = 0;
             break;
 
@@ -737,15 +747,15 @@ int wolfSSH_ConvertConsole(WOLFSSH* ssh, WOLFSSH_HANDLE handle, byte* buf,
 
                 case 'D':
                     /* linefeed */
-                    if (WS_WRITECONSOLE(handle, "\n", sizeof("\n"), &wrt, NULL)
+                    if (WS_WRITECONSOLE(handle, "\n", 1, &wrt, NULL)
                         == 0) {
                         WLOG(WS_LOG_DEBUG, "Error writing newline to handle");
                         return WS_FATAL_ERROR;
                     }
                     break;
 
                 case 'E': /* newline */
-                    if (WS_WRITECONSOLE(handle, "\n", sizeof("\n"), &wrt, NULL)
+                    if (WS_WRITECONSOLE(handle, "\n", 1, &wrt, NULL)
                             == 0) {
                         WLOG(WS_LOG_DEBUG, "Error writing newline to handle");
                         return WS_FATAL_ERROR;

tests/api.c

@@ -3336,6 +3336,8 @@ struct RealPathTestFailCase realPathFail[] = {
     { "12345678", "12345678", 8, WS_INVALID_PATH_E },
     /* Copy segment will not fit in output. */
     { "1234567", "12345678", 8, WS_INVALID_PATH_E },
+    /* Separator plus segment must leave room for the NUL. */
+    { NULL, "aaa/bbb", 8, WS_INVALID_PATH_E },
 };
 
 static void DoRealPathTestFailCase(struct RealPathTestFailCase* tc)