Re: [PATCH v2] fast-import: disallow "." and ".." path components

From: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>
Date: Nov 27, 2024, 08:28 UTC
Message-ID: <79f6027b-123c-40a6-975b-1c6053b9391c@app.fastmail.com>
In-Reply-To: <pull.1831.v2.git.1732561248717.gitgitgadget@gmail.com>

Hi. I see that this is in `next` now so the following might be irrelevant.

On Mon, Nov 25, 2024, at 20:00, Elijah Newren via GitGitGadget wrote:

Show 12 quoted lines

> From: Elijah Newren <newren@gmail.com>
> [...]
> diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> index 76d5c20f141..995ef76f9d6 100644
> --- a/builtin/fast-import.c
> +++ b/builtin/fast-import.c
> @@ -1466,6 +1466,8 @@ 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 (is_dot_or_dotdot(e->name->str_dat))
> +		die("path %s contains invalid component", p);

Nit: single-quoting the path seems more common:

    $ git grep "\"path '%s'" ':!po/' | wc -l
    17
    $ git grep "\"path %s" ':!po/' | wc -l
    4

Show 25 quoted lines

>  	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

Maybe the test could be parameterized so that both `..` and `.` can be tested? Like in `test_path_eol_success`.

-- 
Kristoffer Haugsbakk