| Message ID | pull.1831.git.1732557520428.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fast-import: disallow "." and ".." path components | expand |
On Mon, Nov 25, 2024 at 12:58 PM Elijah Newren via GitGitGadget <gitgitgadget@gmail.com> wrote: > If a user specified e.g. > M 100644 :1 ../some-file > then fast-import previously would happily create a git history where > there is a tree in the top-level directory named "..", and with a file > inside that directory named "some-file". The top-level ".." directory > causes problems. While git checkout will die with errors and fsck will > report hasDotdot problems, the user is going to have problems trying to > remove the problematic file. Simply avoid creating this bad history in > the first place. > > Signed-off-by: Elijah Newren <newren@gmail.com> > --- > diff --git a/builtin/fast-import.c b/builtin/fast-import.c > @@ -1466,6 +1466,9 @@ static int tree_content_set( > e->name = to_atom(p, n); > + if (!strcmp(e->name->str_dat, ".") || !strcmp(e->name->str_dat, "..")) { > + die("path %s contains invalid component", p); > + } Probably not worth a reroll, but is_dot_or_dotdot() might be usable here. (And -- style nit -- the braces could be dropped.)
On Mon, Nov 25, 2024 at 10:15 AM Eric Sunshine <sunshine@sunshineco.com> wrote: > > On Mon, Nov 25, 2024 at 12:58 PM Elijah Newren via GitGitGadget > <gitgitgadget@gmail.com> wrote: > > If a user specified e.g. > > M 100644 :1 ../some-file > > then fast-import previously would happily create a git history where > > there is a tree in the top-level directory named "..", and with a file > > inside that directory named "some-file". The top-level ".." directory > > causes problems. While git checkout will die with errors and fsck will > > report hasDotdot problems, the user is going to have problems trying to > > remove the problematic file. Simply avoid creating this bad history in > > the first place. > > > > Signed-off-by: Elijah Newren <newren@gmail.com> > > --- > > diff --git a/builtin/fast-import.c b/builtin/fast-import.c > > @@ -1466,6 +1466,9 @@ static int tree_content_set( > > e->name = to_atom(p, n); > > + if (!strcmp(e->name->str_dat, ".") || !strcmp(e->name->str_dat, "..")) { > > + die("path %s contains invalid component", p); > > + } > > Probably not worth a reroll, but is_dot_or_dotdot() might be usable here. > > (And -- style nit -- the braces could be dropped.) Good catches, thanks. I think they are worth a reroll; I'll send one in.
diff --git a/builtin/fast-import.c b/builtin/fast-import.c index 76d5c20f141..e0e0ca3657a 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -1466,6 +1466,9 @@ static int tree_content_set( root->tree = t = grow_tree_content(t, t->entry_count); e = new_tree_entry(); e->name = to_atom(p, n); + if (!strcmp(e->name->str_dat, ".") || !strcmp(e->name->str_dat, "..")) { + die("path %s contains invalid component", p); + } e->versions[0].mode = 0; oidclr(&e->versions[0].oid, the_repository->hash_algo); t->entries[t->entry_count++] = e; diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index 6224f54d4d2..caf3dc003a0 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -522,6 +522,26 @@ test_expect_success 'B: fail on invalid committer (5)' ' test_must_fail git fast-import <input ' +test_expect_success 'B: fail on invalid file path' ' + cat >input <<-INPUT_END && + blob + mark :1 + data <<EOF + File contents + EOF + + commit refs/heads/badpath + committer Name <email> $GIT_COMMITTER_DATE + data <<COMMIT + Commit Message + COMMIT + M 100644 :1 ../invalid-path + INPUT_END + + test_when_finished "git update-ref -d refs/heads/badpath" && + test_must_fail git fast-import <input +' + ### ### series C ###