tidy clang-tidy

apps/wolfsshd/configuration.c

@@ -91,8 +91,16 @@ struct WOLFSSHD_CONFIG {
     byte authKeysFileSet:1; /* if not set then no explicit authorized keys */
 };
 
-int CountWhitespace(const char* in, int inSz, byte inv);
-int SetFileString(char** dst, const char* src, void* heap);
+/* Maximum depth of nested Include directives. Bounds the recursion
+ * through wolfSSHD_ConfigLoad -> ParseConfigLine -> HandleConfigOption
+ * -> HandleInclude -> wolfSSHD_ConfigLoad. */
+#ifndef WOLFSSHD_MAX_INCLUDE_DEPTH
+#define WOLFSSHD_MAX_INCLUDE_DEPTH 16
+#endif
+static int ConfigLoad(WOLFSSHD_CONFIG* conf, const char* filename, int depth);
+
+static int CountWhitespace(const char* in, int inSz, byte inv);
+static int SetFileString(char** dst, const char* src, void* heap);
 
 /* convert a string into seconds, handles if 'm' for minutes follows the string
  * number, i.e. 2m
@@ -616,7 +624,8 @@ static int HandlePort(WOLFSSHD_CONFIG* conf, const char* value)
     return ret;
 }
 
-static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value)
+/* NOLINTNEXTLINE(misc-no-recursion): bounded by WOLFSSHD_MAX_INCLUDE_DEPTH */
+static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value, int depth)
 {
     const char *ptr;
     const char *ptr2;
@@ -802,7 +811,7 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value)
                                 WSNPRINTF(filepath, PATH_MAX, "%s/%s", path,
                                          fileNames[i]);
                             }
-                            ret = wolfSSHD_ConfigLoad(conf, filepath);
+                            ret = ConfigLoad(conf, filepath, depth);
                             if (ret != WS_SUCCESS) {
                                 break;
                             }
@@ -834,7 +843,7 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value)
 #endif
         }
         else {
-            ret = wolfSSHD_ConfigLoad(conf, value);
+            ret = ConfigLoad(conf, value, depth);
         }
     }
     return ret;
@@ -974,8 +983,9 @@ static int HandleForcedCommand(WOLFSSHD_CONFIG* conf, const char* value,
 }
 
 /* returns WS_SUCCESS on success */
+/* NOLINTNEXTLINE(misc-no-recursion): bounded by WOLFSSHD_MAX_INCLUDE_DEPTH */
 static int HandleConfigOption(WOLFSSHD_CONFIG** conf, int opt,
-        const char* value, const char* full, int fullSz)
+        const char* value, const char* full, int fullSz, int depth)
 {
     int ret = WS_BAD_ARGUMENT;
 
@@ -1043,7 +1053,7 @@ static int HandleConfigOption(WOLFSSHD_CONFIG** conf, int opt,
             ret = WS_SUCCESS;
             break;
         case OPT_INCLUDE:
-            ret = HandleInclude(*conf, value);
+            ret = HandleInclude(*conf, value, depth);
             break;
         case OPT_CHROOT_DIR:
             ret = HandleChrootDir(*conf, value);
@@ -1074,7 +1084,7 @@ static int HandleConfigOption(WOLFSSHD_CONFIG** conf, int opt,
 
 /* helper function to count white spaces, returns the number of white spaces on
  * success */
-int CountWhitespace(const char* in, int inSz, byte inv)
+static int CountWhitespace(const char* in, int inSz, byte inv)
 {
     int i = 0;
 
@@ -1100,8 +1110,9 @@ int CountWhitespace(const char* in, int inSz, byte inv)
  * Fails if any option is found that is unknown/unsupported
  * Match command will create new configs for specific matching cases
  */
+/* NOLINTNEXTLINE(misc-no-recursion): bounded by WOLFSSHD_MAX_INCLUDE_DEPTH */
 WOLFSSHD_STATIC int ParseConfigLine(WOLFSSHD_CONFIG** conf, const char* l,
-                                    int lSz)
+                                    int lSz, int depth)
 {
     int ret = WS_BAD_ARGUMENT;
     int sz  = 0;
@@ -1132,7 +1143,8 @@ WOLFSSHD_STATIC int ParseConfigLine(WOLFSSHD_CONFIG** conf, const char* l,
         else {
             WMEMCPY(tmp, l + idx, sz);
             tmp[sz] = 0;
-            ret = HandleConfigOption(conf, found->tag, tmp, l + idx, lSz - idx);
+            ret = HandleConfigOption(conf,
+                    found->tag, tmp, l + idx, lSz - idx, depth);
         }
     }
     else {
@@ -1153,6 +1165,13 @@ WOLFSSHD_STATIC int ParseConfigLine(WOLFSSHD_CONFIG** conf, const char* l,
  * returns WS_SUCCESS on success
  */
 int wolfSSHD_ConfigLoad(WOLFSSHD_CONFIG* conf, const char* filename)
+{
+    return ConfigLoad(conf, filename, 0);
+}
+
+
+/* NOLINTNEXTLINE(misc-no-recursion): bounded by WOLFSSHD_MAX_INCLUDE_DEPTH */
+static int ConfigLoad(WOLFSSHD_CONFIG* conf, const char* filename, int depth)
 {
     WFILE *f;
     WOLFSSHD_CONFIG* currentConfig;
@@ -1163,12 +1182,20 @@ int wolfSSHD_ConfigLoad(WOLFSSHD_CONFIG* conf, const char* filename)
     if (conf == NULL || filename == NULL)
         return BAD_FUNC_ARG;
 
+    if (depth >= WOLFSSHD_MAX_INCLUDE_DEPTH) {
+        wolfSSH_Log(WS_LOG_ERROR,
+                "[SSHD] Include depth (%d) exceeded loading %s",
+                WOLFSSHD_MAX_INCLUDE_DEPTH, filename);
+        return WS_BAD_ARGUMENT;
+    }
+
     if (WFOPEN(NULL, &f, filename, "rb") != 0) {
         wolfSSH_Log(WS_LOG_ERROR, "Unable to open SSHD config file %s",
                 filename);
         return BAD_FUNC_ARG;
     }
     wolfSSH_Log(WS_LOG_INFO, "[SSHD] parsing config file %s", filename);
+    depth++;
 
     currentConfig = conf;
     while ((current = XFGETS(buf, MAX_LINE_SIZE, f)) != NULL) {
@@ -1189,7 +1216,7 @@ int wolfSSHD_ConfigLoad(WOLFSSHD_CONFIG* conf, const char* filename)
             continue; /* commented out line */
         }
 
-        ret = ParseConfigLine(&currentConfig, current, currentSz);
+        ret = ParseConfigLine(&currentConfig, current, currentSz, depth);
         if (ret != WS_SUCCESS) {
             fprintf(stderr, "Unable to parse config line : %s\n", current);
             break;
@@ -1358,7 +1385,7 @@ char* wolfSSHD_ConfigGetUserCAKeysFile(const WOLFSSHD_CONFIG* conf)
     return ret;
 }
 
-int SetFileString(char** dst, const char* src, void* heap)
+static int SetFileString(char** dst, const char* src, void* heap)
 {
     int ret = WS_SUCCESS;
 

apps/wolfsshd/configuration.h

@@ -59,7 +59,7 @@ WOLFSSHD_CONFIG* wolfSSHD_GetUserConf(const WOLFSSHD_CONFIG* conf,
 void wolfSSHD_ConfigSavePID(const WOLFSSHD_CONFIG* conf);
 
 #ifdef WOLFSSHD_UNIT_TEST
-int ParseConfigLine(WOLFSSHD_CONFIG** conf, const char* l, int lSz);
+int ParseConfigLine(WOLFSSHD_CONFIG** conf, const char* l, int lSz, int depth);
 #endif
 
 #endif /* WOLFSSHD_H */

apps/wolfsshd/test/test_configuration.c

@@ -35,7 +35,7 @@
 #endif /* __GNUC__ */
 
 
-void Log(const char *const, ...) FMTCHECK;
+void Log(const char *const fmt, ...) FMTCHECK;
 void Log(const char *const fmt, ...)
 {
     va_list vlist;
@@ -261,7 +261,7 @@ static int test_ParseConfigLine(void)
             Log("    Testing scenario: %s.", vectors[i].desc);
 
             ret = ParseConfigLine(&conf, vectors[i].line,
-                                  (int)WSTRLEN(vectors[i].line));
+                                  (int)WSTRLEN(vectors[i].line), 0);
 
             if ((ret == WS_SUCCESS && !vectors[i].shouldFail) ||
                 (ret != WS_SUCCESS && vectors[i].shouldFail)) {
@@ -293,7 +293,7 @@ static int test_ConfigCopy(void)
     conf = head;
 
     /* string fields via ParseConfigLine */
-#define PCL(s) ParseConfigLine(&conf, s, (int)WSTRLEN(s))
+#define PCL(s) ParseConfigLine(&conf, s, (int)WSTRLEN(s), 0)
     if (ret == WS_SUCCESS) ret = PCL("Banner /etc/issue");
     if (ret == WS_SUCCESS) ret = PCL("ChrootDirectory /var/chroot");
     if (ret == WS_SUCCESS) ret = PCL("HostKey /etc/ssh/ssh_host_key");
@@ -406,6 +406,82 @@ static int test_ConfigCopy(void)
     return ret;
 }
 
+/* Bounded recursion through Include directives: a self-including config
+ * must fail with WS_BAD_ARGUMENT once the depth limit is hit, and the
+ * config object must remain usable so a subsequent load of a normal
+ * config on the same WOLFSSHD_CONFIG still succeeds. */
+static int test_IncludeRecursionBound(void)
+{
+    int ret = WS_SUCCESS;
+    WOLFSSHD_CONFIG* conf = NULL;
+    WFILE* f = NULL;
+    const char* loopPath = "./include_loop.conf";
+    const char* normalPath = "./include_normal.conf";
+    const char* loopContents = "Include ./include_loop.conf\n";
+    const char* normalContents = "Port 22\n";
+    word32 sz, wr;
+
+    WFOPEN(NULL, &f, loopPath, "w");
+    if (f == NULL) {
+        Log("    Could not create %s.\n", loopPath);
+        return WS_FATAL_ERROR;
+    }
+    sz = (word32)WSTRLEN(loopContents);
+    wr = (word32)WFWRITE(NULL, loopContents, sizeof(char), sz, f);
+    WFCLOSE(NULL, f);
+    if (sz != wr) {
+        WREMOVE(0, loopPath);
+        return WS_FATAL_ERROR;
+    }
+
+    WFOPEN(NULL, &f, normalPath, "w");
+    if (f == NULL) {
+        WREMOVE(0, loopPath);
+        Log("    Could not create %s.\n", normalPath);
+        return WS_FATAL_ERROR;
+    }
+    sz = (word32)WSTRLEN(normalContents);
+    wr = (word32)WFWRITE(NULL, normalContents, sizeof(char), sz, f);
+    WFCLOSE(NULL, f);
+    if (sz != wr) {
+        WREMOVE(0, loopPath);
+        WREMOVE(0, normalPath);
+        return WS_FATAL_ERROR;
+    }
+
+    conf = wolfSSHD_ConfigNew(NULL);
+    if (conf == NULL) {
+        ret = WS_MEMORY_E;
+    }
+
+    if (ret == WS_SUCCESS) {
+        Log("    Testing scenario: self-include hits depth bound.");
+        if (wolfSSHD_ConfigLoad(conf, loopPath) == WS_BAD_ARGUMENT) {
+            Log(" PASSED.\n");
+        }
+        else {
+            Log(" FAILED.\n");
+            ret = WS_FATAL_ERROR;
+        }
+    }
+
+    if (ret == WS_SUCCESS) {
+        Log("    Testing scenario: config reusable after failed include.");
+        if (wolfSSHD_ConfigLoad(conf, normalPath) == WS_SUCCESS) {
+            Log(" PASSED.\n");
+        }
+        else {
+            Log(" FAILED.\n");
+            ret = WS_FATAL_ERROR;
+        }
+    }
+
+    wolfSSHD_ConfigFree(conf);
+    WREMOVE(0, loopPath);
+    WREMOVE(0, normalPath);
+    return ret;
+}
+
 /* Verifies ConfigFree releases all string fields - most useful under ASan. */
 static int test_ConfigFree(void)
 {
@@ -418,7 +494,7 @@ static int test_ConfigFree(void)
         ret = WS_MEMORY_E;
     conf = head;
 
-#define PCL(s) ParseConfigLine(&conf, s, (int)WSTRLEN(s))
+#define PCL(s) ParseConfigLine(&conf, s, (int)WSTRLEN(s), 0)
     if (ret == WS_SUCCESS) ret = PCL("Banner /etc/issue");
     if (ret == WS_SUCCESS) ret = PCL("ChrootDirectory /var/chroot");
     if (ret == WS_SUCCESS) ret = PCL("HostKey /etc/ssh/ssh_host_key");
@@ -707,6 +783,7 @@ const TEST_CASE testCases[] = {
     TEST_DECL(test_ConfigDefaults),
     TEST_DECL(test_ParseConfigLine),
     TEST_DECL(test_ConfigCopy),
+    TEST_DECL(test_IncludeRecursionBound),
     TEST_DECL(test_ConfigFree),
 #ifdef WOLFSSL_BASE64_ENCODE
     TEST_DECL(test_CheckAuthKeysLine),

apps/wolfsshd/wolfsshd.c

@@ -203,8 +203,6 @@ static void ShowUsage(void)
 static void interruptCatch(int in)
 {
     (void)in;
-    if (logFile)
-        fprintf(logFile, "Closing down wolfSSHD\n");
     quit = 1;
 }
 
@@ -2753,6 +2751,9 @@ static int StartSSHD(int argc, char** argv)
             }
 #endif
         }
+        if (quit && logFile) {
+            fprintf(logFile, "Closing down wolfSSHD\n");
+        }
     }
 
 #ifdef _WIN32

examples/scpclient/scpclient.h

@@ -22,6 +22,6 @@
 #ifndef _WOLFSSH_EXAMPLES_SCPCLIENT_H_
 #define _WOLFSSH_EXAMPLES_SCPCLIENT_H_
 
-THREAD_RETURN WOLFSSH_THREAD scp_client(void*);
+THREAD_RETURN WOLFSSH_THREAD scp_client(void* args);
 
 #endif /* _WOLFSSH_EXAMPLES_SCPCLIENT_H_ */

examples/sftpclient/sftpclient.c

@@ -105,7 +105,7 @@ static void err_msg(const char* s)
     static word32 startTime;
     #define TIMEOUT_VALUE 120
 
-    word32 current_time(int);
+    word32 current_time(int reset);
 #ifdef USE_WINDOWS_API
     #include <time.h>
     #define WIN32_LEAN_AND_MEAN
@@ -268,7 +268,7 @@ static void sig_handler(const int sig)
     (void)sig;
 
     interrupt = 1;
-    wolfSSH_SFTP_Interrupt(ssh);
+    wolfSSH_SFTP_Interrupt(ssh); /* NOLINT(bugprone-signal-handler) */
 }
 #endif /* WS_NO_SIGNAL */
 

src/internal.c

@@ -7561,7 +7561,6 @@ static int DoUserAuthRequestRsaCert(WOLFSSH* ssh, WS_UserAuthData_PublicKey* pk,
                         publicKeyTypeSz) == 0) {
             sigTypeOk = 1;
         }
-    #ifdef WOLFSSH_CERTS
         else if (publicKeyType != NULL
                     && pk->publicKeyTypeSz == 14
                     && WMEMCMP(pk->publicKeyType,
@@ -7579,7 +7578,6 @@ static int DoUserAuthRequestRsaCert(WOLFSSH* ssh, WS_UserAuthData_PublicKey* pk,
                 sigTypeOk = 1;
             }
         }
-    #endif
         if (!sigTypeOk) {
             WLOG(WS_LOG_DEBUG,
                  "Signature's type does not match public key type");

src/log.c

@@ -50,7 +50,8 @@
 
 
 #ifndef WOLFSSL_NO_DEFAULT_LOGGING_CB
-    static void DefaultLoggingCb(enum wolfSSH_LogLevel, const char *const);
+    static void DefaultLoggingCb(enum wolfSSH_LogLevel level,
+            const char *const msgStr);
     static wolfSSH_LoggingCb logFunction = DefaultLoggingCb;
 #else /* WOLFSSH_NO_DEFAULT_LOGGING_CB */
     static wolfSSH_LoggingCb logFunction = NULL;

src/wolfscp.c

@@ -1078,12 +1078,19 @@ static int GetScpTimestamp(WOLFSSH* ssh, byte* buf, word32 bufSz,
 
     /* skip '0 ' */
     if (ret == WS_SUCCESS) {
-        if (buf[idx] != '0' || ++idx > bufSz)
+        if (buf[idx] != '0') {
             ret = WS_SCP_TIMESTAMP_E;
-
-        if (ret == WS_SUCCESS) {
-            if (buf[idx] != ' ' || ++idx > bufSz)
+        }
+        else {
+            idx++;
+            if (idx >= bufSz || buf[idx] != ' ') {
                 ret = WS_SCP_TIMESTAMP_E;
+            }
+            else {
+                idx++;
+                if (idx >= bufSz)
+                    ret = WS_SCP_TIMESTAMP_E;
+            }
         }
     }