Skip to content

Commit

Permalink
listdir(): reuse a single buffer to store every file name to display
Browse files Browse the repository at this point in the history
Allocating a new buffer for each entry is useless.

And as these buffers are allocated on the stack, on systems with a
small stack size, with many entries, the limit can easily be reached,
causing a stack exhaustion and aborting the user session.

Reported by Antonio Morales from the GitHub Security Lab team, thanks!
  • Loading branch information
jedisct1 committed Dec 30, 2019
1 parent 75ae1c9 commit aea56f4
Showing 1 changed file with 8 additions and 7 deletions.
15 changes: 8 additions & 7 deletions src/ls.c
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,8 @@ static void listdir(unsigned int depth, int f, void * const tls_fd,
char *names;
PureFileInfo *s;
PureFileInfo *r;
char *alloca_subdir;
size_t sizeof_subdir;

This comment has been minimized.

Copy link
@maged9977

maged9977 Jan 12, 2021

j

int d;

if (depth >= max_ls_depth || matches >= max_ls_files) {
Expand Down Expand Up @@ -690,14 +692,12 @@ static void listdir(unsigned int depth, int f, void * const tls_fd,
}
outputfiles(f, tls_fd);
r = dir;
sizeof_subdir = PATH_MAX + 1U;
if ((alloca_subdir = ALLOCA(sizeof_subdir)) == NULL) {
goto toomany;
}
while (opt_R && r != s) {
if (r->name_offset != (size_t) -1 && !chdir(FI_NAME(r))) {
char *alloca_subdir;
const size_t sizeof_subdir = PATH_MAX + 1U;

if ((alloca_subdir = ALLOCA(sizeof_subdir)) == NULL) {
goto toomany;
}
if (SNCHECK(snprintf(alloca_subdir, sizeof_subdir, "%s/%s",
name, FI_NAME(r)), sizeof_subdir)) {
goto nolist;
Expand All @@ -706,8 +706,8 @@ static void listdir(unsigned int depth, int f, void * const tls_fd,
wrstr(f, tls_fd, alloca_subdir);
wrstr(f, tls_fd, ":\r\n\r\n");
listdir(depth + 1U, f, tls_fd, alloca_subdir);

nolist:
ALLOCA_FREE(alloca_subdir);
if (matches >= max_ls_files) {
goto toomany;
}
Expand All @@ -720,6 +720,7 @@ static void listdir(unsigned int depth, int f, void * const tls_fd,
r++;
}
toomany:
ALLOCA_FREE(alloca_subdir);
free(names);
free(dir);
names = NULL;
Expand Down

0 comments on commit aea56f4

Please sign in to comment.