diff --git a/apps/wolfsshd/configuration.c b/apps/wolfsshd/configuration.c index 91dd6480..9710c683 100644 --- a/apps/wolfsshd/configuration.c +++ b/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(¤tConfig, current, currentSz); + ret = ParseConfigLine(¤tConfig, current, currentSz, depth); if (ret != WS_SUCCESS) { fprintf(stderr, "Unable to parse config line : %s\n", current); break; @@ -1356,7 +1383,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; diff --git a/apps/wolfsshd/configuration.h b/apps/wolfsshd/configuration.h index 064db6bb..d2b49110 100644 --- a/apps/wolfsshd/configuration.h +++ b/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 */ diff --git a/apps/wolfsshd/test/test_configuration.c b/apps/wolfsshd/test/test_configuration.c index 6b9d1eb0..cac8585c 100644 --- a/apps/wolfsshd/test/test_configuration.c +++ b/apps/wolfsshd/test/test_configuration.c @@ -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"); @@ -437,7 +437,7 @@ static int test_GetUserConfMatchOverride(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) /* permissive global settings */ if (ret == WS_SUCCESS) ret = PCL("PasswordAuthentication yes"); if (ret == WS_SUCCESS) ret = PCL("PermitEmptyPasswords yes"); @@ -501,6 +501,82 @@ static int test_GetUserConfMatchOverride(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) { @@ -513,7 +589,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"); @@ -861,6 +937,7 @@ const TEST_CASE testCases[] = { TEST_DECL(test_ConfigCopy), TEST_DECL(test_GetUserConfMatchOverride), TEST_DECL(test_CAKeysFileDiffers), + TEST_DECL(test_IncludeRecursionBound), TEST_DECL(test_ConfigFree), #ifdef WOLFSSL_BASE64_ENCODE TEST_DECL(test_CheckAuthKeysLine),