From 997c1daf1dce036cd52ba2a4a3d4060b74dd9d54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Tue, 25 Aug 2026 20:03:47 +0200 Subject: [PATCH 1/4] worktree add: don't read out of bounds in worktree_basename() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When we search for the start of the basename and `len` is zero, `name` ends up being `path` - 1, out of bounds. Avoid that by checking before decrementing. Fixes https://github.com/git-for-windows/git/issues/6346. Original-patch-by: Matthias Aßhauer Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- builtin/worktree.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index d21c43fde3..1d827c4eae 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; From 382f88157754097baad9fb0b45513d9e9135a63e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Tue, 25 Aug 2026 20:03:48 +0200 Subject: [PATCH 2/4] worktree add: reject separator-only path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit worktree_basename() extracts an empty basename from a path consisting only of zero or more path separators. We can't use that as a worktree name. Properly report such a path as invalid instead of triggering a BUG that asks the user what just happened. Original-patch-by: Matthias Aßhauer Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- builtin/worktree.c | 2 ++ t/t2400-worktree-add.sh | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/builtin/worktree.c b/builtin/worktree.c index 1d827c4eae..214de50d4c 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -491,6 +491,8 @@ static int add_worktree(const char *path, const char *refname, name = worktree_basename(path, &len); strbuf_add(&sb, name, path + len - name); + if (!sb.len) + die(_("invalid path '%s'"), path); sanitize_refname_component(sb.buf, &sb_name); if (!sb_name.len) BUG("How come '%s' becomes empty after sanitization?", sb.buf); diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh index 58b4445cc4..4ffdd56fb4 100755 --- a/t/t2400-worktree-add.sh +++ b/t/t2400-worktree-add.sh @@ -46,6 +46,10 @@ test_expect_success '"add" refuses to checkout locked branch' ' test_path_is_missing .git/worktrees/zere ' +test_expect_success '"add" rejects an empty path' ' + test_must_fail git worktree add "" HEAD +' + test_expect_success 'checking out paths not complaining about linked checkouts' ' ( cd existing_empty && From a6de316efde124a132d38e50c6407477df47f9cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Tue, 25 Aug 2026 20:03:49 +0200 Subject: [PATCH 3/4] worktree add: trim slashes when deriving branch name from path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit worktree_basename() sets `n` to the length of `path` without trailing path separators, not to the length of the basename. This matters when deriving a branch name from a path with more than one component. E.g.: path: /new/worktree/ s: ^ n: |-----------| So here xstrndup(s, n) copies up to 13 characters from "worktree/", effectively to the end of the string, including the trailing dash. Path separators are not allowed at the end of branch names, so strip them off by calculating the basename length and extracting just that part. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- builtin/worktree.c | 4 ++-- t/t2400-worktree-add.sh | 13 +++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index 214de50d4c..e5b7d6f5ec 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -768,7 +768,7 @@ static char *dwim_branch(const char *path, char **new_branch) int n; int branch_exists; const char *s = worktree_basename(path, &n); - char *branchname = xstrndup(s, n); + char *branchname = xmemdupz(s, path + n - s); struct strbuf ref = STRBUF_INIT; branch_exists = !check_branch_ref(&ref, branchname) && @@ -877,7 +877,7 @@ static int add(int ac, const char **av, const char *prefix, if (opts.orphan && !new_branch) { int n; const char *s = worktree_basename(path, &n); - new_branch = new_branch_to_free = xstrndup(s, n); + new_branch = new_branch_to_free = xmemdupz(s, path + n - s); } else if (opts.orphan) { ; /* no-op */ } else if (opts.detach) { diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh index 4ffdd56fb4..9542a17cf3 100755 --- a/t/t2400-worktree-add.sh +++ b/t/t2400-worktree-add.sh @@ -298,6 +298,11 @@ test_expect_success '"add" with omitted' ' test_cmp_rev HEAD bat ' +test_expect_success '"add" with trailing slash and omitted' ' + git worktree add waffle/bit/ && + test_cmp_rev HEAD bit +' + test_expect_success '"add" checks out existing branch of dwimd name' ' git branch dwim HEAD~1 && git worktree add dwim && @@ -388,6 +393,14 @@ test_expect_success '"add --orphan (no -b)"' ' test_cmp expected actual ' +test_expect_success '"add --orphan with trailing slash (no -b)"' ' + test_when_finished "git worktree remove -f -f neworphan" && + git worktree add --orphan ./neworphan/ && + echo refs/heads/neworphan >expected && + git -C neworphan symbolic-ref HEAD >actual && + test_cmp expected actual +' + test_expect_success '"add --orphan --quiet"' ' test_when_finished "git worktree remove -f -f orphandir" && git worktree add --quiet --orphan -b neworphan orphandir 2>log.actual && From 2e8a9d94b009c69628e29bc7c50d9a3fc12e14ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Tue, 25 Aug 2026 20:03:50 +0200 Subject: [PATCH 4/4] worktree add: let worktree_basename() return string copy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit worktree_basename() requires callers to do pointer arithmetic to get the actual basename. Simplify them by doing the calculations in the function and returning a copy of the basename directly. Remind programmers to free the result by renaming the function to worktree_basename_dup(). Among the three callers of the original function, two immediately make copies of the returned string before using and freeing it, which makes for an easy conversion. Convert the other one from resetting a shared strbuf to freeing the allocated string, which requires the same number of lines, but no arithmetic. The added allocation is negligible because it's small and there's only one per run of "git worktree add". Signed-off-by: René Scharfe [jc: rephrased the second paragraph a bit.] Signed-off-by: Junio C Hamano --- builtin/worktree.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index e5b7d6f5ec..e5a4196f7f 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -294,7 +294,7 @@ static void remove_junk_on_signal(int signo) raise(signo); } -static const char *worktree_basename(const char *path, int *olen) +static char *worktree_basename_dup(const char *path) { const char *name; int len; @@ -307,8 +307,7 @@ static const char *worktree_basename(const char *path, int *olen) while (name > path && !is_dir_sep(name[-1])) name--; - *olen = len; - return name; + return xmemdupz(name, path + len - name); } /* check that path is viable location for worktree */ @@ -462,6 +461,7 @@ static int add_worktree(const char *path, const char *refname, struct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT; struct strbuf sb = STRBUF_INIT; const char *name; + char *name_to_free = NULL; struct strvec child_env = STRVEC_INIT; unsigned int counter = 0; int len, ret; @@ -489,14 +489,12 @@ static int add_worktree(const char *path, const char *refname, if (!commit && !opts->orphan) die(_("invalid reference: %s"), refname); - name = worktree_basename(path, &len); - strbuf_add(&sb, name, path + len - name); - if (!sb.len) + name = name_to_free = worktree_basename_dup(path); + if (!*name) die(_("invalid path '%s'"), path); - sanitize_refname_component(sb.buf, &sb_name); + sanitize_refname_component(name, &sb_name); if (!sb_name.len) - BUG("How come '%s' becomes empty after sanitization?", sb.buf); - strbuf_reset(&sb); + BUG("How come '%s' becomes empty after sanitization?", name); name = sb_name.buf; repo_git_path_replace(the_repository, &sb_repo, "worktrees/%s", name); len = sb_repo.len; @@ -629,6 +627,7 @@ done: strbuf_release(&sb_git); strbuf_release(&sb_name); free_worktree(wt); + free(name_to_free); return ret; } @@ -765,10 +764,8 @@ static int dwim_orphan(const struct add_opts *opts, int opt_track, int remote) static char *dwim_branch(const char *path, char **new_branch) { - int n; int branch_exists; - const char *s = worktree_basename(path, &n); - char *branchname = xmemdupz(s, path + n - s); + char *branchname = worktree_basename_dup(path); struct strbuf ref = STRBUF_INIT; branch_exists = !check_branch_ref(&ref, branchname) && @@ -875,9 +872,7 @@ static int add(int ac, const char **av, const char *prefix, } if (opts.orphan && !new_branch) { - int n; - const char *s = worktree_basename(path, &n); - new_branch = new_branch_to_free = xmemdupz(s, path + n - s); + new_branch = new_branch_to_free = worktree_basename_dup(path); } else if (opts.orphan) { ; /* no-op */ } else if (opts.detach) {