Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci_find_file/unix: add fast-path for file lookup #1350

Merged
merged 5 commits into from
Jul 9, 2021
Merged

ci_find_file/unix: add fast-path for file lookup #1350

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8c088f6
ci_find_file/unix: add fast-path for file lookup
rofl0r Jul 8, 2021
1f7c216
ci_find_file: fix memory leaks in error paths
rofl0r Jul 8, 2021
0a9e89d
ci_find_file: add security check to not allow fs traversal
rofl0r Jul 8, 2021
56f1322
ci_find_file: add comment about .. fs traversal check
rofl0r Jul 8, 2021
3951d93
ci_find_file: fix directory handle leak on error
rofl0r Jul 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 26 additions & 8 deletions Common/util/misc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ char *ci_find_file(const char *dir_name, const char *file_name)
}

#else
/* Case Insensitive File Find */
/* Case Sensitive File Find - only used on UNIX platforms */
char *ci_find_file(const char *dir_name, const char *file_name)
{
struct stat statbuf;
Expand Down Expand Up @@ -115,14 +115,30 @@ char *ci_find_file(const char *dir_name, const char *file_name)
fix_filename_slashes(filename);
}

// the ".." check here prevents file system traversal -
// since only in this fast-path it's possible a potentially evil
// script could try to break out of the directories it's restricted
// to, whereas the latter chdir/opendir approach checks file by file
// in the directory. it's theoretically possible a valid filename
// could contain "..", but in that case it will just fallback to the
// slower method later on and succeed.
if(directory && filename && !strstr(filename, "..")) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is a correct check. It's possible to have a file name containing two dots.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, but in that case we fallback to the chdir/opendir approach below

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure!

Copy link
Contributor

@ivan-mogilko ivan-mogilko Jul 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you ) (that's even more elaborate than I thought about)

char buf[1024];
snprintf(buf, sizeof buf, "%s/%s", directory, filename);
lstat(buf, &statbuf);
if (S_ISREG(statbuf.st_mode) || S_ISLNK(statbuf.st_mode)) {
diamond = strdup(buf); goto out;
}
}

if (directory == nullptr) {
char *match = nullptr;
int match_len = 0;
int dir_len = 0;

match = get_filename(filename);
if (match == nullptr)
return nullptr;
goto out;

match_len = strlen(match);
dir_len = (match - filename);
Expand All @@ -143,17 +159,17 @@ char *ci_find_file(const char *dir_name, const char *file_name)

if ((prevdir = opendir(".")) == nullptr) {
fprintf(stderr, "ci_find_file: cannot open current working directory\n");
return nullptr;
goto out;
}

if (chdir(directory) == -1) {
fprintf(stderr, "ci_find_file: cannot change to directory: %s\n", directory);
return nullptr;
goto out_pd;
}

if ((rough = opendir(directory)) == nullptr) {
fprintf(stderr, "ci_find_file: cannot open directory: %s\n", directory);
return nullptr;
goto out_pd;
}

while ((entry = readdir(rough)) != nullptr) {
Expand All @@ -171,11 +187,13 @@ char *ci_find_file(const char *dir_name, const char *file_name)
}
closedir(rough);

out_pd:;
fchdir(dirfd(prevdir));
closedir(prevdir);

free(directory);
free(filename);
out:;
if(directory) free(directory);
if(filename) free(filename);

return diamond;
}
Expand Down