Re: [PATCH v1.5] worktree: Fix out of bounds read that causes data loss and reject invalid empty input in worktree add
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 11, 2026, 22:46 UTC
- Message-ID
- <xmqqwltwz36a.fsf@gitster.g>
- In-Reply-To
- <52ee6501-24ac-402b-b650-92a829030380@web.de>
René Scharfe <l.s.r@web.de> writes:
Show 20 quoted lines
> From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= <mha1993@live.de> > > `worktree_basename` tries to read from memory before the passed `path` > string, if `path` is empty (or only consists of directory separators). > That results in unexpected nonsense data being returned to the caller, > which can lead to issues, such as `git worktree add ""` recursively > deleting the current working directory, including `.git`. > > Stop reading out of bounds in these cases to avoid that behaviour. > > This leads to `git worktree add ""` consistently exiting with the > message `BUG: How come '' becomes empty after sanitization?`, which is > still undesirable, but at least it doesn't result in data loss anymore. > > This fixes https://github.com/git-for-windows/git/issues/6346 > > Signed-off-by: René Scharfe <l.s.r@web.de> > --- > How about this while we're waiting for a reroll? It implements what the > commit message says, nothing more. Follows the style of the first loop.
This one I think is obvious and clear. Why not take the authorship too so that we do not have to worry about DCO?
Show 23 quoted lines
>
> builtin/worktree.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index 654d27c3e1..a770dd5ead 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -303,11 +303,9 @@ static const char *worktree_basename(const char *path, int *olen)
> while (len && is_dir_sep(path[len - 1]))
> len--;
>
> - for (name = path + len - 1; name > path; name--)
> - if (is_dir_sep(*name)) {
> - name++;
> - break;
> - }
> + name = path + len;
> + while (name > path && !is_dir_sep(name[-1]))
> + name--;
>
> *olen = len;
> return name;