From 2ce75c3543bbcd2a1456dbe5f9d4786af8fd7b39 Mon Sep 17 00:00:00 2001 From: Mark Atwood Date: Thu, 9 Jul 2026 17:28:50 -0700 Subject: [PATCH] fix: security hardening across wolfSSH Batch of Fenrir static-analysis findings in the security_sensitive theme: - #1684 src/agent.c: stop logging the plaintext passphrase in the agent PostLock/PostUnlock stubs; log only that a (un)lock was requested and ForceZero the stack buffer before return. - #4108 apps/wolfsshd/configuration.c: QNX Include-dir filter lstat'd dir->d_name (relative to CWD, not the include dir); build the full path with WSNPRINTF and treat snprintf/lstat failure as non-directory so the count and populate passes stay consistent. - #4794 src/wolfterm.c: reset escState to WC_ESC_NONE after the pre-loop CSI/OSC handling so a stale escape state is not carried over, and bounds-guard the buf[i] read in wolfSSH_DoControlSeq. - #5849 apps/wolfsshd/configuration.c: reject symlinks (DT_LNK) at both POSIX Include-dir filter sites (CWE-61) so a symlink in the Include dir is not followed by fopen and parsed as config. - #5852 src/internal.c: in wolfSSH_CleanPath parent-collapse, only match a '..' component that ends at a delimiter or NUL, guarding against '..X'. --- apps/wolfsshd/configuration.c | 32 ++++++++++++++++++++++++-------- src/agent.c | 6 ++++-- src/internal.c | 6 ++++-- src/wolfterm.c | 5 +++++ 4 files changed, 37 insertions(+), 12 deletions(-) diff --git a/apps/wolfsshd/configuration.c b/apps/wolfsshd/configuration.c index c16c63b16..6b042d4a3 100644 --- a/apps/wolfsshd/configuration.c +++ b/apps/wolfsshd/configuration.c @@ -787,11 +787,20 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value, int depth) /* Skip sub-directories */ #if defined(__QNX__) || defined(__QNXNTO__) struct stat s; - - lstat(dir->d_name, &s); - if (!S_ISDIR(s.st_mode)) + int qnxRet = WSNPRINTF(filepath, PATH_MAX, "%s/%s", + path, dir->d_name); + + /* Full path is needed: lstat resolves relative to CWD, + * not the include dir. On snprintf/lstat failure treat + * as a non-directory so both passes agree. */ + if (qnxRet < 0 || qnxRet >= PATH_MAX || + lstat(filepath, &s) != 0 || + !S_ISDIR(s.st_mode)) #else - if (dir->d_type != DT_DIR) + /* Skip sub-directories and symlinks (CWE-61: a + * symlink in the Include dir would otherwise be + * followed by fopen and parsed as config). */ + if (dir->d_type != DT_DIR && dir->d_type != DT_LNK) #endif { fileCount++; @@ -819,11 +828,18 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value, int depth) /* Skip sub-directories */ #if defined(__QNX__) || defined(__QNXNTO__) struct stat s; - - lstat(dir->d_name, &s); - if (!S_ISDIR(s.st_mode)) + int qnxRet = WSNPRINTF(filepath, PATH_MAX, "%s/%s", + path, dir->d_name); + + /* Full path is needed: lstat resolves relative to + * CWD, not the include dir. On snprintf/lstat + * failure treat as a non-directory so both passes + * agree. */ + if (qnxRet < 0 || qnxRet >= PATH_MAX || + lstat(filepath, &s) != 0 || + !S_ISDIR(s.st_mode)) #else - if (dir->d_type != DT_DIR) + if (dir->d_type != DT_DIR && dir->d_type != DT_LNK) #endif { /* Duplicate the name; readdir() may reuse its diff --git a/src/agent.c b/src/agent.c index 7b7ac5c5d..28d9b6883 100644 --- a/src/agent.c +++ b/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 requested"); + 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 requested"); + ForceZero(pp, sizeof(pp)); return WS_SUCCESS; } diff --git a/src/internal.c b/src/internal.c index 8b7fd28cc..445b2a495 100644 --- a/src/internal.c +++ b/src/internal.c @@ -19705,8 +19705,10 @@ 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] == '.') { + /* if next two chars are .. (and the component ends there) + * then delete; guard against '..X' components */ + 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 */ diff --git a/src/wolfterm.c b/src/wolfterm.c index 7ddd5d117..0be38d0a9 100644 --- a/src/wolfterm.c +++ b/src/wolfterm.c @@ -473,6 +473,9 @@ static int wolfSSH_DoControlSeq(WOLFSSH* ssh, WOLFSSH_HANDLE handle, byte* buf, } else { numArgs = getArgs(buf, bufSz, &i, args); + if (i >= bufSz) { + return WS_FATAL_ERROR; + } c = buf[i]; i++; } } @@ -670,6 +673,7 @@ int wolfSSH_ConvertConsole(WOLFSSH* ssh, WOLFSSH_HANDLE handle, byte* buf, return ret; } ssh->escBufSz = 0; + ssh->escState = WC_ESC_NONE; break; case WS_ESC_OSC: @@ -678,6 +682,7 @@ int wolfSSH_ConvertConsole(WOLFSSH* ssh, WOLFSSH_HANDLE handle, byte* buf, return ret; } ssh->escBufSz = 0; + ssh->escState = WC_ESC_NONE; break; default: