Directory traversal fixes
cover Directory traversal fixes
This is a resend of the final two patches of my series, as per
https://lore.kernel.org/git/[email protected]/. It will not merge cleanly unless the last two patches of en/dir-traversal-fixes are first reverted, as Junio suggested he'd be doing.
(Corrected Reviewed-by -> Signed-off-by in the first patch, minor stylistic fix for the second.)
Derrick Stolee (1):
dir: update stale description of treat_directory()
Elijah Newren (1):
dir: introduce readdir_skip_dot_and_dotdot() helper
builtin/clean.c | 4 +--- builtin/worktree.c | 4 +--- diff-no-index.c | 5 ++--- dir.c | 38 ++++++++++++++++++++++---------------- dir.h | 2 ++ entry.c | 5 +---- notes-merge.c | 5 +---- object-file.c | 4 +--- packfile.c | 5 +---- rerere.c | 4 +--- worktree.c | 12 +++--------- 11 files changed, 36 insertions(+), 52 deletions(-)
base-commit: dd55fc0df15c338a8dbec6dec6ca6b58edc8acef
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-961%2Fnewren%2Fdirectory-traversal-fixes-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-961/newren/directory-traversal-fixes-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/961
Signature · 1 line
gitgitgadget
PATCH 1/2 dir: update stale description of treat_directory()
From: Derrick Stolee <[email protected]>
The documentation comment for treat_directory() was originally written in 095952 (Teach directory traversal about subprojects, 2007-04-11) which was before the 'struct dir_struct' split its bitfield of named options into a 'flags' enum in 7c4c97c0 (Turn the flags in struct dir_struct into a single variable, 2009-02-16). When those flags changed, the comment became stale, since members like 'show_other_directories' transitioned into flags like DIR_SHOW_OTHER_DIRECTORIES.
Update the comments for treat_directory() to use these flag names rather than the old member names.
--- dir.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
dir.c +6 -7
@@ -1740,13 +1740,13 @@ static enum exist_status directory_exists_in_index(struct index_state *istate,* Case 3: if we didn't have it in the index previously, we * have a few sub-cases: * -* (a) if "show_other_directories" is true, we show it as -* just a directory, unless "hide_empty_directories" is +* (a) if DIR_SHOW_OTHER_DIRECTORIES flag is set, we show it as +* just a directory, unless DIR_HIDE_EMPTY_DIRECTORIES is * also true, in which case we need to check if it contains any * untracked and / or ignored files. -* (b) if it looks like a git directory, and we don't have -* 'no_gitlinks' set we treat it as a gitlink, and show it -* as a directory. +* (b) if it looks like a git directory and we don't have the +* DIR_NO_GITLINKS flag, then we treat it as a gitlink, and +* show it as a directory. * (c) otherwise, we recurse into it. */ static enum path_treatment treat_directory(struct dir_struct *dir,
@@ -1834,7 +1834,6 @@ static enum path_treatment treat_directory(struct dir_struct *dir,return path_recurse; } -/* This is the "show_other_directories" case */ assert (dir->flags &DIR_SHOW_OTHER_DIRECTORIES );/*
@@ -1849,7 +1848,7 @@ static enum path_treatment treat_directory(struct dir_struct *dir,/* Special cases for where this directory is excluded/ignored */ if (excluded) {/* -* In the show_other_directories case, if we're not +* If DIR_SHOW_OTHER_DIRECTORIES is set and we're not * hiding empty directories, there is no need to * recurse into an ignored directory. */
Signature · 1 line
gitgitgadget
PATCH 2/2 dir: introduce readdir_skip_dot_and_dotdot() helper
In reply to Elijah Newren via GitGitGadgetFrom: Elijah Newren <[email protected]>
Many places in the code were doing
while ((d = readdir(dir)) != NULL) {
if (is_dot_or_dotdot(d->d_name))
continue;
...process d...
}
Introduce a readdir_skip_dot_and_dotdot() helper to make that a one-liner:
while ((d = readdir_skip_dot_and_dotdot(dir)) != NULL) {
...process d...
}This helper particularly simplifies checks for empty directories.
Also use this helper in read_cached_dir() so that our statistics are consistent across platforms. (In other words, read_cached_dir() should have been using is_dot_or_dotdot() and skipping such entries, but did not and left it to treat_path() to detect and mark such entries as path_none.)
--- builtin/clean.c | 4 +--- builtin/worktree.c | 4 +--- diff-no-index.c | 5 ++--- dir.c | 25 ++++++++++++++++--------- dir.h | 2 ++ entry.c | 5 +---- notes-merge.c | 5 +---- object-file.c | 4 +--- packfile.c | 5 +---- rerere.c | 4 +--- worktree.c | 12 +++--------- 11 files changed, 30 insertions(+), 45 deletions(-)
builtin/clean.c +1 -3
@@ -189,10 +189,8 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,strbuf_complete (path,'/' ); len = path->len; -while ((e =readdir (dir)) !=NULL ) { +while ((e =readdir_skip_dot_and_dotdot (dir)) !=NULL ) {struct stat st; -if (is_dot_or_dotdot (e->d_name)) -continue ;strbuf_setlen (path, len);strbuf_addstr (path, e->d_name);
builtin/worktree.c +1 -3
@@ -118,10 +118,8 @@ static void prune_worktrees(void)struct dirent *d;if (!dir)return ; -while ((d =readdir (dir)) !=NULL ) { +while ((d =readdir_skip_dot_and_dotdot (dir)) !=NULL ) {char *path; -if (is_dot_or_dotdot (d->d_name)) -continue ;strbuf_reset (&reason);if (should_prune_worktree (d->d_name, &reason, &path, expire))prune_worktree (d->d_name, reason.buf);
diff-no-index.c +2 -3
@@ -26,9 +26,8 @@ static int read_directory_contents(const char *path, struct string_list *list)if (!(dir =opendir (path)))return error ("Could not open directory %s" , path); -while ((e =readdir (dir))) -if (!is_dot_or_dotdot (e->d_name)) -string_list_insert (list, e->d_name); +while ((e =readdir_skip_dot_and_dotdot (dir))) +string_list_insert (list, e->d_name);closedir (dir);return 0 ;
dir.c +16 -9
@@ -59,6 +59,17 @@ void dir_init(struct dir_struct *dir)memset (dir,0 ,sizeof (*dir)); } +struct dirent *readdir_skip_dot_and_dotdot (DIR *dirp) +{ +struct dirent *e; + +while ((e =readdir (dirp)) !=NULL ) { +if (!is_dot_or_dotdot (e->d_name)) +break ; + } +return e; +} +int count_slashes (const char *s) {int cnt =0 ;
@@ -2332,7 +2343,7 @@ static int read_cached_dir(struct cached_dir *cdir)struct dirent *de;if (cdir->fdir) { - de =readdir (cdir->fdir); + de =readdir_skip_dot_and_dotdot (cdir->fdir);if (!de) { cdir->d_name =NULL ; cdir->d_type =DT_UNKNOWN ;
@@ -2931,11 +2942,9 @@ int is_empty_dir(const char *path)if (!dir)return 0 ; -while ((e =readdir (dir)) !=NULL ) -if (!is_dot_or_dotdot (e->d_name)) { - ret =0 ; -break ; - } + e =readdir_skip_dot_and_dotdot (dir); +if (e) + ret =0 ;closedir (dir);return ret;
@@ -2975,10 +2984,8 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)strbuf_complete (path,'/' ); len = path->len; -while ((e =readdir (dir)) !=NULL ) { +while ((e =readdir_skip_dot_and_dotdot (dir)) !=NULL ) {struct stat st; -if (is_dot_or_dotdot (e->d_name)) -continue ;strbuf_setlen (path, len);strbuf_addstr (path, e->d_name);
dir.h +2 -0
@@ -342,6 +342,8 @@ struct dir_struct {unsigned visited_directories; }; +struct dirent *readdir_skip_dot_and_dotdot (DIR *dirp); +/*Count the number of slashes for string s*/ int count_slashes (const char *s);
entry.c +1 -4
@@ -56,12 +56,9 @@ static void remove_subtree(struct strbuf *path)if (!dir)die_errno ("cannot opendir '%s'" , path->buf); -while ((de =readdir (dir)) !=NULL ) { +while ((de =readdir_skip_dot_and_dotdot (dir)) !=NULL ) {struct stat st; -if (is_dot_or_dotdot (de->d_name)) -continue ; -strbuf_addch (path,'/' );strbuf_addstr (path, de->d_name);if (lstat (path->buf, &st))
notes-merge.c +1 -4
@@ -695,13 +695,10 @@ int notes_merge_commit(struct notes_merge_options *o,strbuf_addch (&path,'/' ); baselen = path.len; -while ((e =readdir (dir)) !=NULL ) { +while ((e =readdir_skip_dot_and_dotdot (dir)) !=NULL ) {struct stat st;struct object_id obj_oid, blob_oid; -if (is_dot_or_dotdot (e->d_name)) -continue ; -if (get_oid_hex (e->d_name, &obj_oid)) {if (o->verbosity >=3 ) printf("Skipping non-SHA1 entry '%s%s'\n",
object-file.c +1 -3
@@ -2304,10 +2304,8 @@ int for_each_file_in_obj_subdir(unsigned int subdir_nr,strbuf_addch (path,'/' ); baselen = path->len; -while ((de =readdir (dir))) { +while ((de =readdir_skip_dot_and_dotdot (dir))) {size_t namelen; -if (is_dot_or_dotdot (de->d_name)) -continue ; namelen =strlen (de->d_name);strbuf_setlen (path, baselen);
packfile.c +1 -4
@@ -813,10 +813,7 @@ void for_each_file_in_pack_dir(const char *objdir, }strbuf_addch (&path,'/' ); dirnamelen = path.len; -while ((de =readdir (dir)) !=NULL ) { -if (is_dot_or_dotdot (de->d_name)) -continue ; - +while ((de =readdir_skip_dot_and_dotdot (dir)) !=NULL ) {strbuf_setlen (&path, dirnamelen);strbuf_addstr (&path, de->d_name);
rerere.c +1 -3
@@ -1190,13 +1190,11 @@ void rerere_gc(struct repository *r, struct string_list *rr)if (!dir)die_errno (_ ("unable to open rr-cache directory" ));/* Collect stale conflict IDs ... */ -while ((e =readdir (dir))) { +while ((e =readdir_skip_dot_and_dotdot (dir))) {struct rerere_dir *rr_dir;struct rerere_id id;int now_empty; -if (is_dot_or_dotdot (e->d_name)) -continue ;if (!is_rr_cache_dirname (e->d_name))continue ;/* or should we remove e->d_name? */
worktree.c +3 -9
@@ -128,10 +128,8 @@ struct worktree **get_worktrees(void) dir =opendir (path.buf);strbuf_release (&path);if (dir) { -while ((d =readdir (dir)) !=NULL ) { +while ((d =readdir_skip_dot_and_dotdot (dir)) !=NULL ) {struct worktree *linked =NULL ; -if (is_dot_or_dotdot (d->d_name)) -continue ;if ((linked =get_linked_worktree (d->d_name))) {ALLOC_GROW (list, counter +1 , alloc);
@@ -486,13 +484,9 @@ int submodule_uses_worktrees(const char *path)if (!dir)return 0 ; -while ((d =readdir (dir)) !=NULL ) { -if (is_dot_or_dotdot (d->d_name)) -continue ; - + d =readdir_skip_dot_and_dotdot (dir); +if (d !=NULL ) ret =1 ; -break ; - }closedir (dir);return ret; }
Signature · 1 line
gitgitgadget