Skip to content

Commit 06215f8

Browse files
wolfsshd: fix uninitialized fileNames[] deref in HandleInclude
1 parent e2bfa19 commit 06215f8

1 file changed

Lines changed: 31 additions & 5 deletions

File tree

apps/wolfsshd/configuration.c

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -723,7 +723,7 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value, int depth)
723723

724724
if (ret == WS_SUCCESS) {
725725
if (!WOPENDIR(NULL, conf->heap, &d, path)) {
726-
word32 fileCount = 0, i, j;
726+
word32 fileCount = 0, fileFilled = 0, i, j;
727727
char** fileNames = NULL;
728728

729729
/* Count up the number of files */
@@ -749,6 +749,12 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value, int depth)
749749
if (fileNames == NULL) {
750750
ret = WS_MEMORY_E;
751751
}
752+
else {
753+
/* Zero so any slot not filled by the second pass
754+
* (e.g. files removed from the directory between
755+
* the two passes) is NULL rather than garbage. */
756+
WMEMSET(fileNames, 0, fileCount * sizeof(char*));
757+
}
752758
}
753759

754760
if (ret == WS_SUCCESS) {
@@ -764,21 +770,33 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value, int depth)
764770
if (dir->d_type != DT_DIR)
765771
#endif
766772
{
773+
/* Duplicate the name; readdir() may reuse its
774+
* dirent storage on the next call, so the
775+
* pointer cannot be retained across the loop. */
776+
char* nameCopy = WSTRDUP(dir->d_name, conf->heap,
777+
DYNTYPE_PATH);
778+
if (nameCopy == NULL) {
779+
ret = WS_MEMORY_E;
780+
break;
781+
}
767782
/* Insert in string order */
768783
for (j = 0; j < i; j++) {
769-
if (WSTRCMP(dir->d_name, fileNames[j])
770-
< 0) {
784+
if (WSTRCMP(nameCopy, fileNames[j]) < 0) {
771785
WMEMMOVE(fileNames+j+1, fileNames+j,
772786
(i - j)*sizeof(char*));
773787
break;
774788
}
775789
}
776-
fileNames[j] = dir->d_name;
790+
fileNames[j] = nameCopy;
777791
i++;
778792
}
779793
}
794+
/* Only process slots actually filled by the second
795+
* pass; the directory may have shrunk since the
796+
* count pass. */
797+
fileFilled = i;
780798

781-
for (i = 0; i < fileCount; i++) {
799+
for (i = 0; ret == WS_SUCCESS && i < fileFilled; i++) {
782800
/* Check if filename prefix matches */
783801
if (prefixLen > 0) {
784802
if ((int)WSTRLEN(fileNames[i]) <= prefixLen) {
@@ -817,6 +835,14 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value, int depth)
817835
}
818836
}
819837

838+
/* Free the duplicated names. fileFilled counts the
839+
* slots actually populated, so every entry below it
840+
* holds a valid pointer. */
841+
for (i = 0; i < fileFilled; i++) {
842+
if (fileNames[i] != NULL) {
843+
WFREE(fileNames[i], conf->heap, DYNTYPE_PATH);
844+
}
845+
}
820846
if (fileNames != NULL) {
821847
WFREE(fileNames, conf->heap, DYNTYPE_PATH);
822848
}

0 commit comments

Comments
 (0)