Skip to content

Commit ef56aca

Browse files
yosuke-wolfsslejohnstown
authored andcommitted
wolfsshd: fix uninitialized fileNames[] deref in HandleInclude
1 parent 253c157 commit ef56aca

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
@@ -749,7 +749,7 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value, int depth)
749749

750750
if (ret == WS_SUCCESS) {
751751
if (!WOPENDIR(NULL, conf->heap, &d, path)) {
752-
word32 fileCount = 0, i, j;
752+
word32 fileCount = 0, fileFilled = 0, i, j;
753753
char** fileNames = NULL;
754754

755755
/* Count up the number of files */
@@ -775,6 +775,12 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value, int depth)
775775
if (fileNames == NULL) {
776776
ret = WS_MEMORY_E;
777777
}
778+
else {
779+
/* Zero so any slot not filled by the second pass
780+
* (e.g. files removed from the directory between
781+
* the two passes) is NULL rather than garbage. */
782+
WMEMSET(fileNames, 0, fileCount * sizeof(char*));
783+
}
778784
}
779785

780786
if (ret == WS_SUCCESS) {
@@ -790,21 +796,33 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value, int depth)
790796
if (dir->d_type != DT_DIR)
791797
#endif
792798
{
799+
/* Duplicate the name; readdir() may reuse its
800+
* dirent storage on the next call, so the
801+
* pointer cannot be retained across the loop. */
802+
char* nameCopy = WSTRDUP(dir->d_name, conf->heap,
803+
DYNTYPE_PATH);
804+
if (nameCopy == NULL) {
805+
ret = WS_MEMORY_E;
806+
break;
807+
}
793808
/* Insert in string order */
794809
for (j = 0; j < i; j++) {
795-
if (WSTRCMP(dir->d_name, fileNames[j])
796-
< 0) {
810+
if (WSTRCMP(nameCopy, fileNames[j]) < 0) {
797811
WMEMMOVE(fileNames+j+1, fileNames+j,
798812
(i - j)*sizeof(char*));
799813
break;
800814
}
801815
}
802-
fileNames[j] = dir->d_name;
816+
fileNames[j] = nameCopy;
803817
i++;
804818
}
805819
}
820+
/* Only process slots actually filled by the second
821+
* pass; the directory may have shrunk since the
822+
* count pass. */
823+
fileFilled = i;
806824

807-
for (i = 0; i < fileCount; i++) {
825+
for (i = 0; ret == WS_SUCCESS && i < fileFilled; i++) {
808826
/* Check if filename prefix matches */
809827
if (prefixLen > 0) {
810828
if ((int)WSTRLEN(fileNames[i]) <= prefixLen) {
@@ -842,6 +860,14 @@ static int HandleInclude(WOLFSSHD_CONFIG *conf, const char *value, int depth)
842860
}
843861
}
844862

863+
/* Free the duplicated names. fileFilled counts the
864+
* slots actually populated, so every entry below it
865+
* holds a valid pointer. */
866+
for (i = 0; i < fileFilled; i++) {
867+
if (fileNames[i] != NULL) {
868+
WFREE(fileNames[i], conf->heap, DYNTYPE_PATH);
869+
}
870+
}
845871
if (fileNames != NULL) {
846872
WFREE(fileNames, conf->heap, DYNTYPE_PATH);
847873
}

0 commit comments

Comments
 (0)