From 85069b816e6e7af30cda1f6fbd7a64aca2042498 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 25 Aug 2022 10:58:10 -0700 Subject: [PATCH 1/7] Wildcard Config 1. Fix the wildcard config file include. 2. Update the guard flags so macOS can also use wildcards. 3. Change the user priviledge separating setting to a bitfield. 4. Add test_configuration test to gitignore. --- .gitignore | 1 + apps/wolfsshd/configuration.c | 7 ++++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 8db4d74d..86575574 100644 --- a/.gitignore +++ b/.gitignore @@ -58,6 +58,7 @@ examples/scpclient/wolfscp # applications apps/wolfsshd/wolfsshd +apps/wolfsshd/test/test_configuration # test output tests/*.test diff --git a/apps/wolfsshd/configuration.c b/apps/wolfsshd/configuration.c index 31178a22..a7c85e4f 100644 --- a/apps/wolfsshd/configuration.c +++ b/apps/wolfsshd/configuration.c @@ -59,7 +59,7 @@ struct WOLFSSHD_CONFIG { char* authKeysFile; long loginTimer; word16 port; - byte usePrivilegeSeparation; + byte usePrivilegeSeparation:2; byte passwordAuth:1; byte pubKeyAuth:1; byte permitRootLogin:1; @@ -445,7 +445,7 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value) /* Ignore trailing whitespace */ ptr = value + WSTRLEN(value) - 1; while (ptr != value) { - if (!WISSPACE(*ptr)) { + if (WISSPACE(*ptr)) { ptr--; } else { @@ -474,7 +474,8 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value) /* Use wildcard */ if (found) { -#ifdef __unix__ +#if defined(__unix__) || defined(__unix) || \ + (defined(__APPLE__) && defined(__MACH__)) int ret; struct dirent *dir; DIR *d; From c42f8fc2fe42e45a29b3b62b215d1b3d4acbf2b6 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 25 Aug 2022 14:08:34 -0700 Subject: [PATCH 2/7] Wildcard Config 1. Process the included config files in strcmp order. --- apps/wolfsshd/configuration.c | 108 ++++++++++++++++++++++------------ 1 file changed, 70 insertions(+), 38 deletions(-) diff --git a/apps/wolfsshd/configuration.c b/apps/wolfsshd/configuration.c index a7c85e4f..88244f01 100644 --- a/apps/wolfsshd/configuration.c +++ b/apps/wolfsshd/configuration.c @@ -506,47 +506,79 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value) d = opendir(path); if (d) { + word32 fileCount = 0, i, j; + char** fileNames = NULL; + + /* Count up the number of files */ while ((dir = readdir(d)) != NULL) { - if (dir->d_type == DT_DIR) { - /* Skip sub-directories */ - continue; - } - else { - /* Check if filename prefix matches */ - if (prefixLen > 0) { - if ((int)WSTRLEN(dir->d_name) <= prefixLen) { - continue; - } - if (WSTRNCMP(dir->d_name, prefix, prefixLen) != 0) { - continue; - } - } - if (postfix) { - /* Skip if file is too short */ - if (WSTRLEN(dir->d_name) <= WSTRLEN(postfix)) { - continue; - } - if (WSTRNCMP(dir->d_name + WSTRLEN(dir->d_name) - - WSTRLEN(postfix), postfix, WSTRLEN(postfix)) - == 0) { - snprintf(filepath, PATH_MAX, "%s/%s", path, - dir->d_name); - } - else { - /* Not a match */ - continue; - } - } - else { - snprintf(filepath, PATH_MAX, "%s/%s", path, - dir->d_name); - } - ret = wolfSSHD_ConfigLoad(conf, filepath); - if (ret != WS_SUCCESS) { - return ret; - } + /* Skip sub-directories */ + if (dir->d_type != DT_DIR) { + fileCount++; } } + rewinddir(d); + + if (fileCount > 0) { + fileNames = (char**)WMALLOC(fileCount * sizeof(char*), NULL, 0); + } + + i = 0; + while ((dir = readdir(d)) != NULL && i < fileCount) { + /* Skip sub-directories */ + if (dir->d_type != DT_DIR) { + /* Insert in string order */ + for (j = 0; j < i; j++) { + if (WSTRCMP(dir->d_name, fileNames[j]) < 0) { + WMEMMOVE(fileNames+j+1, fileNames+j, + (i - j)*sizeof(char*)); + break; + } + } + fileNames[j] = dir->d_name; + i++; + } + } + + for (i = 0; i < fileCount; i++) { + /* Check if filename prefix matches */ + if (prefixLen > 0) { + if ((int)WSTRLEN(fileNames[i]) <= prefixLen) { + continue; + } + if (WSTRNCMP(fileNames[i], prefix, prefixLen) != 0) { + continue; + } + } + if (postfix) { + /* Skip if file is too short */ + if (WSTRLEN(fileNames[i]) <= WSTRLEN(postfix)) { + continue; + } + if (WSTRNCMP(fileNames[i] + WSTRLEN(fileNames[i]) - + WSTRLEN(postfix), postfix, WSTRLEN(postfix)) + == 0) { + snprintf(filepath, PATH_MAX, "%s/%s", path, + fileNames[i]); + } + else { + /* Not a match */ + continue; + } + } + else { + snprintf(filepath, PATH_MAX, "%s/%s", path, + fileNames[i]); + } + ret = wolfSSHD_ConfigLoad(conf, filepath); + if (ret != WS_SUCCESS) { + closedir(d); + WFREE(fileNames, NULL, 0); + WFREE(filepath, NULL, 0); + return ret; + } + } + WFREE(fileNames, NULL, 0); + closedir(d); } else { /* Bad directory */ From a01d31592c45cfbc1d62787e72f119a871da7e8e Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 26 Aug 2022 12:41:00 -0700 Subject: [PATCH 3/7] Wildcard 1. Revise to use the porting functions. 2. Add test cases to check the config wildcards. 3. Generate test files for the wildcard test, and delete them after. --- apps/wolfsshd/configuration.c | 22 +++---- apps/wolfsshd/include.am | 2 +- apps/wolfsshd/test/test_configuration.c | 85 +++++++++++++++++++++++++ wolfssh/port.h | 5 +- 4 files changed, 100 insertions(+), 14 deletions(-) diff --git a/apps/wolfsshd/configuration.c b/apps/wolfsshd/configuration.c index 88244f01..6d39866e 100644 --- a/apps/wolfsshd/configuration.c +++ b/apps/wolfsshd/configuration.c @@ -35,6 +35,7 @@ #include #include #include +#include #include #include @@ -478,7 +479,7 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value) (defined(__APPLE__) && defined(__MACH__)) int ret; struct dirent *dir; - DIR *d; + WDIR d; char *path; char *filepath = (char*)WMALLOC(PATH_MAX, NULL, 0); @@ -504,26 +505,25 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value) prefixLen = (int)(ptr - value); } - d = opendir(path); - if (d) { + if (!WOPENDIR(NULL, conf->heap, &d, path)) { word32 fileCount = 0, i, j; char** fileNames = NULL; /* Count up the number of files */ - while ((dir = readdir(d)) != NULL) { + while ((dir = WREADDIR(&d)) != NULL) { /* Skip sub-directories */ if (dir->d_type != DT_DIR) { fileCount++; } } - rewinddir(d); + WREWINDDIR(&d); if (fileCount > 0) { fileNames = (char**)WMALLOC(fileCount * sizeof(char*), NULL, 0); } i = 0; - while ((dir = readdir(d)) != NULL && i < fileCount) { + while ((dir = WREADDIR(&d)) != NULL && i < fileCount) { /* Skip sub-directories */ if (dir->d_type != DT_DIR) { /* Insert in string order */ @@ -557,7 +557,7 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value) if (WSTRNCMP(fileNames[i] + WSTRLEN(fileNames[i]) - WSTRLEN(postfix), postfix, WSTRLEN(postfix)) == 0) { - snprintf(filepath, PATH_MAX, "%s/%s", path, + WSNPRINTF(filepath, PATH_MAX, "%s/%s", path, fileNames[i]); } else { @@ -566,19 +566,19 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value) } } else { - snprintf(filepath, PATH_MAX, "%s/%s", path, + WSNPRINTF(filepath, PATH_MAX, "%s/%s", path, fileNames[i]); } ret = wolfSSHD_ConfigLoad(conf, filepath); if (ret != WS_SUCCESS) { - closedir(d); + WCLOSEDIR(&d); WFREE(fileNames, NULL, 0); WFREE(filepath, NULL, 0); return ret; } } WFREE(fileNames, NULL, 0); - closedir(d); + WCLOSEDIR(&d); } else { /* Bad directory */ @@ -759,7 +759,7 @@ int wolfSSHD_ConfigLoad(WOLFSSHD_CONFIG* conf, const char* filename) f = XFOPEN(filename, "rb"); if (f == XBADFILE) { - wolfSSH_Log(WS_LOG_ERROR, "Unable to open SSHD config file %s\n", + wolfSSH_Log(WS_LOG_ERROR, "Unable to open SSHD config file %s", filename); return BAD_FUNC_ARG; } diff --git a/apps/wolfsshd/include.am b/apps/wolfsshd/include.am index cc20dee1..9342a24e 100644 --- a/apps/wolfsshd/include.am +++ b/apps/wolfsshd/include.am @@ -15,7 +15,7 @@ apps_wolfsshd_test_test_configuration_SOURCES = apps/wolfsshd/test/test_configur apps/wolfsshd/auth.c apps_wolfsshd_test_test_configuration_LDADD = src/libwolfssh.la apps_wolfsshd_test_test_configuration_DEPENDENCIES = src/libwolfssh.la -apps_wolfsshd_test_test_configuration_CPPFLAGS = -DWOLFSSH_SSHD -DWOLFSSHD_UNIT_TEST -Iapps/wolfsshd/ +apps_wolfsshd_test_test_configuration_CPPFLAGS = $(AM_CPPFLAGS) -DWOLFSSH_SSHD -DWOLFSSHD_UNIT_TEST -Iapps/wolfsshd/ DISTCLEANFILES+= apps/wolfsshd/.libs/wolfsshd \ apps/wolfsshd/test/.libs/test_configuration diff --git a/apps/wolfsshd/test/test_configuration.c b/apps/wolfsshd/test/test_configuration.c index 4b1fe5b7..a17429e2 100644 --- a/apps/wolfsshd/test/test_configuration.c +++ b/apps/wolfsshd/test/test_configuration.c @@ -26,6 +26,76 @@ void Log(const char *const fmt, ...) va_end(vlist); } +static void CleanupWildcardTest(void) +{ + WDIR dir; + struct dirent* d; + char filepath[MAX_PATH*2]; /* d_name is max_path long */ + + if (!WOPENDIR(NULL, NULL, &dir, "./sshd_config.d/")) { + while ((d = WREADDIR(&dir)) != NULL) { + if (d->d_type != DT_DIR) { + WSNPRINTF(filepath, sizeof filepath, "%s%s", + "./sshd_config.d/", d->d_name); + WREMOVE(0, filepath); + } + } + WCLOSEDIR(&dir); + WRMDIR(0, "./sshd_config.d/"); + } +} + +static int SetupWildcardTest(void) +{ + WFILE* f; + const byte fileIds[] = { 0, 1, 50, 59, 99 }; + word32 fileIdsSz = (word32)(sizeof(fileIds) / sizeof(byte)); + word32 i; + int ret; + char filepath[MAX_PATH]; + + ret = WMKDIR(0, "./sshd_config.d/", 0755); + + if (ret == 0) { + for (i = 0; i < fileIdsSz; i++) { + if (fileIds[i] != 0) { + WSNPRINTF(filepath, sizeof filepath, "%s%02u-test.conf", + "./sshd_config.d/", fileIds[i]); + } + else { + WSNPRINTF(filepath, sizeof filepath, "%stest.bad", + "./sshd_config.d/"); + } + + WFOPEN(&f, filepath, "w"); + if (f) { + word32 sz, wr; + char contents[20]; + WSNPRINTF(contents, sizeof contents, "LoginGraceTime %02u", + fileIds[i]); + sz = (word32)WSTRLEN(contents); + wr = (word32)WFWRITE(contents, sizeof(char), sz, f); + WFCLOSE(f); + if (sz != wr) { + Log("Couldn't write the contents of file %s\n", filepath); + ret = -1; + break; + } + } + else { + Log("Couldn't create the file %s\n", filepath); + ret = -1; + break; + } + } + } + else { + Log("Couldn't make the test config directory\n"); + } + + return ret; +} + typedef int (*TEST_FUNC)(void); typedef struct { const char *name; @@ -110,6 +180,13 @@ static int test_ParseConfigLine(void) {"Password auth no", "PasswordAuthentication no", 0}, {"Password auth yes", "PasswordAuthentication yes", 0}, {"Password auth invalid", "PasswordAuthentication wolfsshd", 1}, + + /* Include files tests. */ + {"Include file bad", "Include sshd_config.d/test.bad", 1}, + {"Include file exists", "Include sshd_config.d/01-test.conf", 0}, + {"Include file DNE", "Include sshd_config.d/test-dne.conf", 1}, + {"Include wildcard exists", "Include sshd_config.d/*.conf", 0}, + {"Include wildcard NDE", "Include sshd_config.d/*.dne", 0}, }; const int numVectors = (int)(sizeof(vectors) / sizeof(*vectors)); @@ -153,6 +230,12 @@ int main(int argc, char** argv) (void)argc; (void)argv; + CleanupWildcardTest(); + ret = SetupWildcardTest(); + if (ret != 0) { + return 1; + } + for (i = 0; i < TEST_CASE_CNT; ++i) { ret = RunTest(&testCases[i]); if (ret != WS_SUCCESS) { @@ -160,5 +243,7 @@ int main(int argc, char** argv) } } + CleanupWildcardTest(); + return ret; } diff --git a/wolfssh/port.h b/wolfssh/port.h index 4645599a..4a48db2d 100644 --- a/wolfssh/port.h +++ b/wolfssh/port.h @@ -1217,8 +1217,9 @@ extern "C" { /* returns 0 on success */ #define WOPENDIR(fs,h,c,d) ((*(c) = opendir((d))) == NULL) - #define WCLOSEDIR(d) closedir(*(d)) - #define WREADDIR(d) readdir(*(d)) + #define WCLOSEDIR(d) closedir(*(d)) + #define WREADDIR(d) readdir(*(d)) + #define WREWINDDIR(d) rewinddir(*(d)) #endif /* NO_WOLFSSH_DIR */ #endif #endif /* WOLFSSH_SFTP or WOLFSSH_SCP */ From c90a235cc6e56f74184bce4317ed147fcfc32053 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Sat, 27 Aug 2022 19:16:23 -0700 Subject: [PATCH 4/7] Wildcard 1. Add a guard around the directory porting wrappers for wolfSSHd. --- wolfssh/port.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/wolfssh/port.h b/wolfssh/port.h index 4a48db2d..5327d3ea 100644 --- a/wolfssh/port.h +++ b/wolfssh/port.h @@ -340,7 +340,8 @@ extern "C" { #define WCHMOD(fs,f,m) _chmod((f),(m)) #endif - #if (defined(WOLFSSH_SCP) || defined(WOLFSSH_SFTP)) && \ + #if (defined(WOLFSSH_SCP) || \ + defined(WOLFSSH_SFTP) || defined(WOLFSSH_SSHD)) && \ !defined(WOLFSSH_SCP_USER_CALLBACKS) #ifdef USE_WINDOWS_API @@ -474,8 +475,9 @@ extern "C" { #define WLOCALTIME(c,r) (localtime_r((c),(r))!=NULL) #endif -#if (defined(WOLFSSH_SFTP) || defined(WOLFSSH_SCP)) && \ - !defined(NO_WOLFSSH_SERVER) && !defined(NO_FILESYSTEM) +#if (defined(WOLFSSH_SFTP) || \ + defined(WOLFSSH_SCP) || defined(WOLFSSH_SSHD)) && \ + !defined(NO_WOLFSSH_SERVER) && !defined(NO_FILESYSTEM) #ifndef SIZEOF_OFF_T /* if not using autoconf then make a guess on off_t size based on sizeof From 4d43d8406ed50768789aa0b72d1b1a6fa67c6ee6 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 29 Aug 2022 13:45:45 -0700 Subject: [PATCH 5/7] Wildcard Config 1. Update WMALLOCs to use the config's heap and proper DYNTYPE values. 2. Covert more STDC functions to use the wrapper macros. 3. Check some missed return values on mallocs, and make sure to free all allocated buffers. --- apps/wolfsshd/configuration.c | 42 +++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/apps/wolfsshd/configuration.c b/apps/wolfsshd/configuration.c index 6d39866e..f6e3352d 100644 --- a/apps/wolfsshd/configuration.c +++ b/apps/wolfsshd/configuration.c @@ -481,7 +481,11 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value) struct dirent *dir; WDIR d; char *path; - char *filepath = (char*)WMALLOC(PATH_MAX, NULL, 0); + char *filepath = (char*)WMALLOC(PATH_MAX, conf->heap, DYNTYPE_PATH); + + if (filepath == NULL) { + return WS_MEMORY_E; + } /* Back find the full path */ while (ptr2 != value) { @@ -492,14 +496,23 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value) } if (ptr2 != value) { - path = (char*)WMALLOC(ptr2 - value + 1, NULL, 0); - memcpy(path, value, ptr2 - value); + path = (char*)WMALLOC(ptr2 - value + 1, conf->heap, DYNTYPE_PATH); + if (path == NULL) { + WFREE(filepath, conf->heap, DYNTYPE_PATH); + return WS_MEMORY_E; + } + WMEMCPY(path, value, ptr2 - value); path[ptr2 - value] = '\0'; prefix = ptr2 + 1; prefixLen = (int)(ptr - ptr2 - 1); - } else { - path = (char*)WMALLOC(2, NULL, 0); - memcpy(path, ".", 1); + } + else { + path = (char*)WMALLOC(2, conf->heap, DYNTYPE_PATH); + if (path == NULL) { + WFREE(filepath, conf->heap, DYNTYPE_PATH); + return WS_MEMORY_E; + } + WMEMCPY(path, ".", 1); path[1] = '\0'; prefix = value; prefixLen = (int)(ptr - value); @@ -519,7 +532,8 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value) WREWINDDIR(&d); if (fileCount > 0) { - fileNames = (char**)WMALLOC(fileCount * sizeof(char*), NULL, 0); + fileNames = (char**)WMALLOC(fileCount * sizeof(char*), + conf->heap, DYNTYPE_PATH); } i = 0; @@ -572,20 +586,24 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value) ret = wolfSSHD_ConfigLoad(conf, filepath); if (ret != WS_SUCCESS) { WCLOSEDIR(&d); - WFREE(fileNames, NULL, 0); - WFREE(filepath, NULL, 0); + WFREE(fileNames, conf->heap, DYNTYPE_PATH); + WFREE(filepath, conf->heap, DYNTYPE_PATH); + WFREE(path, conf->heap, DYNTYPE_PATH); return ret; } } - WFREE(fileNames, NULL, 0); + WFREE(fileNames, conf->heap, DYNTYPE_PATH); + WFREE(path, conf->heap, DYNTYPE_PATH); WCLOSEDIR(&d); } else { /* Bad directory */ - WFREE(filepath, NULL, 0); + WFREE(filepath, conf->heap, DYNTYPE_PATH); + WFREE(path, conf->heap, DYNTYPE_PATH); return WS_BAD_ARGUMENT; } - WFREE(filepath, NULL, 0); + WFREE(filepath, conf->heap, DYNTYPE_PATH); + WFREE(path, conf->heap, DYNTYPE_PATH); #else (void)postfix; (void)prefixLen; From 76417aca8893eb4432f176c662a77b5d57066e88 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 29 Aug 2022 17:12:55 -0700 Subject: [PATCH 6/7] Wildcard Config 1. Modify HandleInclude() to have a single return point, and minimize the places where free() needs to be called. 2. Modify the config test wildcard include test file creation to return error similar other test functions. 3. Fix leak of the test configuration object in the wolfSSHd configuration test. --- apps/wolfsshd/configuration.c | 317 +++++++++++++----------- apps/wolfsshd/test/test_configuration.c | 19 +- 2 files changed, 178 insertions(+), 158 deletions(-) diff --git a/apps/wolfsshd/configuration.c b/apps/wolfsshd/configuration.c index f6e3352d..dc92b82c 100644 --- a/apps/wolfsshd/configuration.c +++ b/apps/wolfsshd/configuration.c @@ -437,185 +437,204 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value) const char *prefix = NULL; int prefixLen = 0; int found = 0; + int ret = WS_SUCCESS; /* No value, nothing to do */ if (!value || value[0] == '\0') { - return WS_BAD_ARGUMENT; + ret = WS_BAD_ARGUMENT; } - /* Ignore trailing whitespace */ - ptr = value + WSTRLEN(value) - 1; - while (ptr != value) { - if (WISSPACE(*ptr)) { - ptr--; - } - else { - break; - } - } - - /* Find wildcards */ - ptr2 = ptr; - while (ptr2 != value) { - if (*ptr2 == '*') { - /* Wildcard found */ - found = 1; - if (ptr != ptr2) { - postfix = ptr2 + 1; + if (ret == WS_SUCCESS) { + /* Ignore trailing whitespace */ + ptr = value + WSTRLEN(value) - 1; + while (ptr != value) { + if (WISSPACE(*ptr)) { + ptr--; + } + else { + break; } - break; - } - if (*ptr2 == '/') { - /* Found slash before wildcard directory-wildcards not supported */ - break; - } - ptr2--; - } - ptr = ptr2; - - /* Use wildcard */ - if (found) { -#if defined(__unix__) || defined(__unix) || \ - (defined(__APPLE__) && defined(__MACH__)) - int ret; - struct dirent *dir; - WDIR d; - char *path; - char *filepath = (char*)WMALLOC(PATH_MAX, conf->heap, DYNTYPE_PATH); - - if (filepath == NULL) { - return WS_MEMORY_E; } - /* Back find the full path */ + /* Find wildcards */ + ptr2 = ptr; while (ptr2 != value) { + if (*ptr2 == '*') { + /* Wildcard found */ + found = 1; + if (ptr != ptr2) { + postfix = ptr2 + 1; + } + break; + } if (*ptr2 == '/') { + /* Found slash before wildcard directory-wildcards + * not supported */ break; } ptr2--; } + ptr = ptr2; - if (ptr2 != value) { - path = (char*)WMALLOC(ptr2 - value + 1, conf->heap, DYNTYPE_PATH); - if (path == NULL) { - WFREE(filepath, conf->heap, DYNTYPE_PATH); - return WS_MEMORY_E; + /* Use wildcard */ + if (found) { +#if defined(__unix__) || defined(__unix) || \ + (defined(__APPLE__) && defined(__MACH__)) + struct dirent *dir; + WDIR d; + char *path = NULL; + char *filepath = (char*)WMALLOC(PATH_MAX, conf->heap, DYNTYPE_PATH); + + if (filepath == NULL) { + ret = WS_MEMORY_E; } - WMEMCPY(path, value, ptr2 - value); - path[ptr2 - value] = '\0'; - prefix = ptr2 + 1; - prefixLen = (int)(ptr - ptr2 - 1); - } - else { - path = (char*)WMALLOC(2, conf->heap, DYNTYPE_PATH); - if (path == NULL) { - WFREE(filepath, conf->heap, DYNTYPE_PATH); - return WS_MEMORY_E; - } - WMEMCPY(path, ".", 1); - path[1] = '\0'; - prefix = value; - prefixLen = (int)(ptr - value); - } - if (!WOPENDIR(NULL, conf->heap, &d, path)) { - word32 fileCount = 0, i, j; - char** fileNames = NULL; - - /* Count up the number of files */ - while ((dir = WREADDIR(&d)) != NULL) { - /* Skip sub-directories */ - if (dir->d_type != DT_DIR) { - fileCount++; + if (ret == WS_SUCCESS) { + /* Back find the full path */ + while (ptr2 != value) { + if (*ptr2 == '/') { + break; + } + ptr2--; } - } - WREWINDDIR(&d); - if (fileCount > 0) { - fileNames = (char**)WMALLOC(fileCount * sizeof(char*), - conf->heap, DYNTYPE_PATH); - } - - i = 0; - while ((dir = WREADDIR(&d)) != NULL && i < fileCount) { - /* Skip sub-directories */ - if (dir->d_type != DT_DIR) { - /* Insert in string order */ - for (j = 0; j < i; j++) { - if (WSTRCMP(dir->d_name, fileNames[j]) < 0) { - WMEMMOVE(fileNames+j+1, fileNames+j, - (i - j)*sizeof(char*)); - break; - } - } - fileNames[j] = dir->d_name; - i++; - } - } - - for (i = 0; i < fileCount; i++) { - /* Check if filename prefix matches */ - if (prefixLen > 0) { - if ((int)WSTRLEN(fileNames[i]) <= prefixLen) { - continue; - } - if (WSTRNCMP(fileNames[i], prefix, prefixLen) != 0) { - continue; - } - } - if (postfix) { - /* Skip if file is too short */ - if (WSTRLEN(fileNames[i]) <= WSTRLEN(postfix)) { - continue; - } - if (WSTRNCMP(fileNames[i] + WSTRLEN(fileNames[i]) - - WSTRLEN(postfix), postfix, WSTRLEN(postfix)) - == 0) { - WSNPRINTF(filepath, PATH_MAX, "%s/%s", path, - fileNames[i]); + if (ptr2 != value) { + path = (char*)WMALLOC(ptr2 - value + 1, + conf->heap, DYNTYPE_PATH); + if (path == NULL) { + ret = WS_MEMORY_E; } else { - /* Not a match */ - continue; + WMEMCPY(path, value, ptr2 - value); + path[ptr2 - value] = '\0'; + prefix = ptr2 + 1; + prefixLen = (int)(ptr - ptr2 - 1); } } else { - WSNPRINTF(filepath, PATH_MAX, "%s/%s", path, - fileNames[i]); - } - ret = wolfSSHD_ConfigLoad(conf, filepath); - if (ret != WS_SUCCESS) { - WCLOSEDIR(&d); - WFREE(fileNames, conf->heap, DYNTYPE_PATH); - WFREE(filepath, conf->heap, DYNTYPE_PATH); - WFREE(path, conf->heap, DYNTYPE_PATH); - return ret; + path = (char*)WMALLOC(2, conf->heap, DYNTYPE_PATH); + if (path == NULL) { + ret = WS_MEMORY_E; + } + else { + WMEMCPY(path, ".", 1); + path[1] = '\0'; + prefix = value; + prefixLen = (int)(ptr - value); + } } } - WFREE(fileNames, conf->heap, DYNTYPE_PATH); - WFREE(path, conf->heap, DYNTYPE_PATH); - WCLOSEDIR(&d); + + if (ret == WS_SUCCESS) { + if (!WOPENDIR(NULL, conf->heap, &d, path)) { + word32 fileCount = 0, i, j; + char** fileNames = NULL; + + /* Count up the number of files */ + while ((dir = WREADDIR(&d)) != NULL) { + /* Skip sub-directories */ + if (dir->d_type != DT_DIR) { + fileCount++; + } + } + WREWINDDIR(&d); + + if (fileCount > 0) { + fileNames = (char**)WMALLOC(fileCount * sizeof(char*), + conf->heap, DYNTYPE_PATH); + if (fileNames == NULL) { + ret = WS_MEMORY_E; + } + } + + if (ret == WS_SUCCESS) { + i = 0; + while (i < fileCount && (dir = WREADDIR(&d)) != NULL) { + /* Skip sub-directories */ + if (dir->d_type != DT_DIR) { + /* Insert in string order */ + for (j = 0; j < i; j++) { + if (WSTRCMP(dir->d_name, fileNames[j]) + < 0) { + WMEMMOVE(fileNames+j+1, fileNames+j, + (i - j)*sizeof(char*)); + break; + } + } + fileNames[j] = dir->d_name; + i++; + } + } + + for (i = 0; i < fileCount; i++) { + /* Check if filename prefix matches */ + if (prefixLen > 0) { + if ((int)WSTRLEN(fileNames[i]) <= prefixLen) { + continue; + } + if (WSTRNCMP(fileNames[i], prefix, prefixLen) + != 0) { + continue; + } + } + if (postfix) { + /* Skip if file is too short */ + if (WSTRLEN(fileNames[i]) <= WSTRLEN(postfix)) { + continue; + } + if (WSTRNCMP(fileNames[i] + + WSTRLEN(fileNames[i]) - + WSTRLEN(postfix), + postfix, WSTRLEN(postfix)) + == 0) { + WSNPRINTF(filepath, PATH_MAX, "%s/%s", path, + fileNames[i]); + } + else { + /* Not a match */ + continue; + } + } + else { + WSNPRINTF(filepath, PATH_MAX, "%s/%s", path, + fileNames[i]); + } + ret = wolfSSHD_ConfigLoad(conf, filepath); + if (ret != WS_SUCCESS) { + break; + } + } + + if (fileNames != NULL) { + WFREE(fileNames, conf->heap, DYNTYPE_PATH); + } + } + WCLOSEDIR(&d); + } + else { + /* Bad directory */ + ret = WS_INVALID_PATH_E; + } + } + if (path != NULL) { + WFREE(path, conf->heap, DYNTYPE_PATH); + } + if (filepath != NULL) { + WFREE(filepath, conf->heap, DYNTYPE_PATH); + } +#else + (void)postfix; + (void)prefixLen; + (void)prefix; + /* Don't support wildcards here */ + ret = WS_BAD_ARGUMENT; +#endif } else { - /* Bad directory */ - WFREE(filepath, conf->heap, DYNTYPE_PATH); - WFREE(path, conf->heap, DYNTYPE_PATH); - return WS_BAD_ARGUMENT; + ret = wolfSSHD_ConfigLoad(conf, value); } - WFREE(filepath, conf->heap, DYNTYPE_PATH); - WFREE(path, conf->heap, DYNTYPE_PATH); -#else - (void)postfix; - (void)prefixLen; - (void)prefix; - /* Don't support wildcards here */ - return WS_BAD_ARGUMENT; -#endif } - else { - return wolfSSHD_ConfigLoad(conf, value); - } - return WS_SUCCESS; + return ret; } /* returns WS_SUCCESS on success */ diff --git a/apps/wolfsshd/test/test_configuration.c b/apps/wolfsshd/test/test_configuration.c index a17429e2..df31cbf7 100644 --- a/apps/wolfsshd/test/test_configuration.c +++ b/apps/wolfsshd/test/test_configuration.c @@ -78,19 +78,20 @@ static int SetupWildcardTest(void) WFCLOSE(f); if (sz != wr) { Log("Couldn't write the contents of file %s\n", filepath); - ret = -1; + ret = WS_FATAL_ERROR; break; } } else { Log("Couldn't create the file %s\n", filepath); - ret = -1; + ret = WS_FATAL_ERROR; break; } } } else { Log("Couldn't make the test config directory\n"); + ret = WS_FATAL_ERROR; } return ret; @@ -213,6 +214,7 @@ static int test_ParseConfigLine(void) break; } } + wolfSSHD_ConfigFree(conf); } return ret; @@ -232,14 +234,13 @@ int main(int argc, char** argv) CleanupWildcardTest(); ret = SetupWildcardTest(); - if (ret != 0) { - return 1; - } - for (i = 0; i < TEST_CASE_CNT; ++i) { - ret = RunTest(&testCases[i]); - if (ret != WS_SUCCESS) { - break; + if (ret == 0) { + for (i = 0; i < TEST_CASE_CNT; ++i) { + ret = RunTest(&testCases[i]); + if (ret != WS_SUCCESS) { + break; + } } } From 79ddd784c7377bd005a8707b0e40bc6fab08b62f Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 29 Aug 2022 17:13:12 -0700 Subject: [PATCH 7/7] Scan-Build Cleaning 1. The portfwd example had a couple spots in command line option processing where atol could get called with a null pointer. --- examples/portfwd/portfwd.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/examples/portfwd/portfwd.c b/examples/portfwd/portfwd.c index 93ee600c..c68fbb91 100644 --- a/examples/portfwd/portfwd.c +++ b/examples/portfwd/portfwd.c @@ -267,6 +267,8 @@ THREAD_RETURN WOLFSSH_THREAD portfwd_worker(void* args) break; case 'f': + if (myoptarg == NULL) + err_sys("null argument found"); fwdFromPort = (word16)atoi(myoptarg); break; @@ -281,6 +283,8 @@ THREAD_RETURN WOLFSSH_THREAD portfwd_worker(void* args) break; case 't': + if (myoptarg == NULL) + err_sys("null argument found"); fwdToPort = (word16)atoi(myoptarg); break;