{"thread":{"id":"66061","subject":"[PATCH 0/2] worktree: Fix out of bounds read that causes data loss and reject invalid empty input in worktree add","startedAt":"2026-07-25T11:19:10Z","lastAt":"2026-08-16T19:30:58Z","messageCount":11,"participants":["Matthias Aßhauer via GitGitGadget","Junio C Hamano","René Scharfe"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"548941","messageId":"pull.2187.git.1784978348.gitgitgadget@gmail.com","threadId":"66061","inReplyTo":null,"subject":"[PATCH 0/2] worktree: Fix out of bounds read that causes data loss and reject invalid empty input in worktree add","fromName":"Matthias Aßhauer via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-25T11:19:05Z","receivedAt":"2026-07-25T11:19:10Z","isPatch":true,"body":"Passing an empty string to git worktree add (typically via an unset\nvariable, e.g. git worktree add \"$UNSET_VAR\" -b tb origin/main) can result\nin BUG: How come '' becomes empty after sanitization? but it can also have\nworse consequences: recursively deleting the current working directory,\nincluding .git. The inconsistent behaviour is caused by worktree_basename\nreading unrelated bytes from the memory before path and passing that back to\nadd_worktree, which can circumvent the check for the BUG call.\n\nMatthias Aßhauer (2):\n worktree: don't read out of bounds\n worktree: reject empty string\n\n builtin/worktree.c | 20 +++++++++++++-------\n 1 file changed, 13 insertions(+), 7 deletions(-)\n\n\nbase-commit: 9a0c4701dcd5725c4184599322b52933ff5005ca\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2187%2Frimrul%2Fworktree-fix-oob-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2187/rimrul/worktree-fix-oob-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/2187\n-- \ngitgitgadget\n"},{"id":"548942","messageId":"8bc69c6b80ed42888327331b1567cecf7225ea7e.1784978348.git.gitgitgadget@gmail.com","threadId":"66061","inReplyTo":"pull.2187.git.1784978348.gitgitgadget@gmail.com","subject":"[PATCH 1/2] worktree: don't read out of bounds","fromName":"Matthias Aßhauer via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-25T11:19:06Z","receivedAt":"2026-07-25T11:19:11Z","isPatch":true,"body":"From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= \n\n`worktree_basename` tries to read from memory before the passed `path`\nstring, if `path` is empty (or only consists of directory separators).\nThat results in unexpected nonsense data being returned to the caller,\nwhich can lead to issues, such as `git worktree add \"\"` recursively\ndeleting the current working directory, including `.git`.\n\nStop reading out of bounds in these cases to avoid that behaviour.\n\nThis leads to `git worktree add \"\"` consistently exiting with the\nmessage `BUG: How come '' becomes empty after sanitization?`, which is\nstill undesirable, but at least it doesn't result in data loss anymore.\n\nThis fixes https://github.com/git-for-windows/git/issues/6346\n\nSigned-off-by: Matthias Aßhauer \n---\n builtin/worktree.c | 18 +++++++++++-------\n 1 file changed, 11 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 4bc7b4f6e7..d8188035db 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -297,17 +297,21 @@ static void remove_junk_on_signal(int signo)\n static const char *worktree_basename(const char *path, int *olen)\n {\n \tconst char *name;\n-\tint len;\n+\tint len, len2;\n \n-\tlen = strlen(path);\n+\tlen2 = len = strlen(path);\n \twhile (len && is_dir_sep(path[len - 1]))\n \t\tlen--;\n \n-\tfor (name = path + len - 1; name > path; name--)\n-\t\tif (is_dir_sep(*name)) {\n-\t\t\tname++;\n-\t\t\tbreak;\n-\t\t}\n+\tif(len) {\n+\t\tfor (name = path + len - 1; name > path; name--)\n+\t\t\tif (is_dir_sep(*name)) {\n+\t\t\t\tname++;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t}\n+\telse\n+\t\tname = path + len2;\n \n \t*olen = len;\n \treturn name;\n-- \ngitgitgadget\n\n"},{"id":"548943","messageId":"ec682d75f3a7848dc36f82cf36bbdff6fd283e2d.1784978348.git.gitgitgadget@gmail.com","threadId":"66061","inReplyTo":"pull.2187.git.1784978348.gitgitgadget@gmail.com","subject":"[PATCH 2/2] worktree: reject empty string","fromName":"Matthias Aßhauer via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-07-25T11:19:07Z","receivedAt":"2026-07-25T11:19:13Z","isPatch":true,"body":"From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= \n\n`git worktree add \"\"` errors out with the message `BUG: How come ''\nbecomes empty after sanitization?`, but not due to a bug in the\nsanitization code. An empty string should remain empty during\nsanitization. Instead reject the argument as invalid user input,\nif it's already empty before sanitization.\n\nSigned-off-by: Matthias Aßhauer \n---\n builtin/worktree.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex d8188035db..113dbf98d3 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -496,6 +496,8 @@ static int add_worktree(const char *path, const char *refname,\n \t\tdie(_(\"invalid reference: %s\"), refname);\n \n \tname = worktree_basename(path, &len);\n+\tif (!len)\n+\t\tdie(_(\"the empty string is not a valid worktree\"));\n \tstrbuf_add(&sb, name, path + len - name);\n \tsanitize_refname_component(sb.buf, &sb_name);\n \tif (!sb_name.len)\n-- \ngitgitgadget\n"},{"id":"548971","messageId":"xmqqbjbvypv3.fsf@gitster.g","threadId":"66061","inReplyTo":"8bc69c6b80ed42888327331b1567cecf7225ea7e.1784978348.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] worktree: don't read out of bounds","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-25T16:51:28Z","receivedAt":"2026-07-25T16:51:31Z","isPatch":true,"body":"\"Matthias Aßhauer via GitGitGadget\" writes:\n\n> `worktree_basename` tries to read from memory before the passed `path`\n> string, if `path` is empty (or only consists of directory separators).\n> That results in unexpected nonsense data being returned to the caller,\n> which can lead to issues, such as `git worktree add \"\"` recursively\n> deleting the current working directory, including `.git`.\n\nOK, so you do want to handle a case where path is something silly\nlike \"///\".\n\n> Stop reading out of bounds in these cases to avoid that behaviour.\n>\n> This leads to `git worktree add \"\"` consistently exiting with the\n> message `BUG: How come '' becomes empty after sanitization?`, which is\n> still undesirable, but at least it doesn't result in data loss anymore.\n\nOK.\n\n> diff --git a/builtin/worktree.c b/builtin/worktree.c\n> index 4bc7b4f6e7..d8188035db 100644\n> --- a/builtin/worktree.c\n> +++ b/builtin/worktree.c\n> @@ -297,17 +297,21 @@ static void remove_junk_on_signal(int signo)\n> static const char *worktree_basename(const char *path, int *olen)\n> {\n> \tconst char *name;\n> -\tint len;\n> +\tint len, len2;\n> \n> -\tlen = strlen(path);\n> +\tlen2 = len = strlen(path);\n> \twhile (len && is_dir_sep(path[len - 1]))\n> \t\tlen--;\n\nThese two 'len' variables should have clear names to distinguish\nwhat each length represents. Rather than introducing a cryptic\n'len2', give it a more meaningful name, and rename 'len' as well if\nnecessary.\n\nI suspect that it is to remember the original length of the 'path'\nbefore stripping the trailing directory separators?\n\n> -\tfor (name = path + len - 1; name > path; name--)\n> -\t\tif (is_dir_sep(*name)) {\n> -\t\t\tname++;\n> -\t\t\tbreak;\n> -\t\t}\n\nWhen 'len' is 0, the original code sets 'name' to '&path[-1]' and\ndoes not enter the loop. However, '*olen' is set to 0, and 'name',\npointing before the start of the string, is returned. If left\nunfixed, callers pass it to xstrndup(), strbuf_add(), and the like,\nreading memory before the start of the string, which is horrible and\nworth fixing.\n\n> +\tif(len) {\n> +\t\tfor (name = path + len - 1; name > path; name--)\n> +\t\t\tif (is_dir_sep(*name)) {\n> +\t\t\t\tname++;\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\t}\n> +\telse\n> +\t\tname = path + len2;\n\nStyle:\n\n (1) Missing SP between 'if' and '(len'.\n\n (2) 'else' sits on the same line as '}' that closes the 'if'\n clause.\n\n (3) When any one branch of an 'if'...'else if'...'else' cascade\n needs a pair of braces to group multiple statements, all other\n branches must use braces as well.\n\nTaken together:\n\n\tif (len) {\n\t\t...\n\t} else {\n\t\t...\n\t}\n\nAs for what the patch intends to do, setting 'name = path + len2'\nwhen 'len' is 0 breaks when 'path' consists only of directory\nseparators (for example, \"/\" or \"///\"), no?\n\nIn that case, 'len2' is positive (for example, 3) while 'len' is 0.\nIn add_worktree(), 'path + len - name' evaluates to (path + 0) -\n(path + 3) = -3. Passed as size_t to strbuf_add(), this wraps\naround to SIZE_MAX - 2 (approx. 18 exabytes), leading to a buffer\nallocation failure or a crash.\n\nRather than calculating 'path - 1' out of bounds or introducing\n'len2', worktree_basename() can simply keep 'name = path' when 'len'\nis 0. Using an integer index loop 'for (int i = len - 1; 0 <= i;\ni--)' avoids pointer arithmetic before the start of the buffer\nentirely, I would think. Or am I missing something?\n\nThanks.\n"},{"id":"549329","messageId":"535547d0-39fc-4c6a-a0bb-2a5f43f265ed@web.de","threadId":"66061","inReplyTo":"xmqqbjbvypv3.fsf@gitster.g","subject":"Re: [PATCH 1/2] worktree: don't read out of bounds","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2026-07-31T05:45:58Z","receivedAt":"2026-07-31T05:46:17Z","isPatch":true,"body":"On 7/25/26 6:51 PM, Junio C Hamano wrote:\n> \"Matthias Aßhauer via GitGitGadget\" writes:\n> \n>> `worktree_basename` tries to read from memory before the passed `path`\n>> string, if `path` is empty (or only consists of directory separators).\n>> That results in unexpected nonsense data being returned to the caller,\n>> which can lead to issues, such as `git worktree add \"\"` recursively\n>> deleting the current working directory, including `.git`.\n> \n> OK, so you do want to handle a case where path is something silly\n> like \"///\".\n> \n>> Stop reading out of bounds in these cases to avoid that behaviour.\n>>\n>> This leads to `git worktree add \"\"` consistently exiting with the\n>> message `BUG: How come '' becomes empty after sanitization?`, which is\n>> still undesirable, but at least it doesn't result in data loss anymore.\n> \n> OK.\n> \n>> diff --git a/builtin/worktree.c b/builtin/worktree.c\n>> index 4bc7b4f6e7..d8188035db 100644\n>> --- a/builtin/worktree.c\n>> +++ b/builtin/worktree.c\n>> @@ -297,17 +297,21 @@ static void remove_junk_on_signal(int signo)\n>> static const char *worktree_basename(const char *path, int *olen)\n>> {\n>> \tconst char *name;\n>> -\tint len;\n>> +\tint len, len2;\n>> \n>> -\tlen = strlen(path);\n>> +\tlen2 = len = strlen(path);\n>> \twhile (len && is_dir_sep(path[len - 1]))\n>> \t\tlen--;\n> \n> These two 'len' variables should have clear names to distinguish\n> what each length represents. Rather than introducing a cryptic\n> 'len2', give it a more meaningful name, and rename 'len' as well if\n> necessary.\n> \n> I suspect that it is to remember the original length of the 'path'\n> before stripping the trailing directory separators?\n> \n>> -\tfor (name = path + len - 1; name > path; name--)\n>> -\t\tif (is_dir_sep(*name)) {\n>> -\t\t\tname++;\n>> -\t\t\tbreak;\n>> -\t\t}\n> \n> When 'len' is 0, the original code sets 'name' to '&path[-1]' and\n> does not enter the loop. However, '*olen' is set to 0, and 'name',\n> pointing before the start of the string, is returned. If left\n> unfixed, callers pass it to xstrndup(), strbuf_add(), and the like,\n> reading memory before the start of the string, which is horrible and\n> worth fixing.\n> \n>> +\tif(len) {\n>> +\t\tfor (name = path + len - 1; name > path; name--)\n>> +\t\t\tif (is_dir_sep(*name)) {\n>> +\t\t\t\tname++;\n>> +\t\t\t\tbreak;\n>> +\t\t\t}\n>> +\t}\n>> +\telse\n>> +\t\tname = path + len2;\n> \n> Style:\n> \n> (1) Missing SP between 'if' and '(len'.\n> \n> (2) 'else' sits on the same line as '}' that closes the 'if'\n> clause.\n> \n> (3) When any one branch of an 'if'...'else if'...'else' cascade\n> needs a pair of braces to group multiple statements, all other\n> branches must use braces as well.\n> \n> Taken together:\n> \n> \tif (len) {\n> \t\t...\n> \t} else {\n> \t\t...\n> \t}\n> \n> As for what the patch intends to do, setting 'name = path + len2'\n> when 'len' is 0 breaks when 'path' consists only of directory\n> separators (for example, \"/\" or \"///\"), no?\n> \n> In that case, 'len2' is positive (for example, 3) while 'len' is 0.\n> In add_worktree(), 'path + len - name' evaluates to (path + 0) -\n> (path + 3) = -3. Passed as size_t to strbuf_add(), this wraps\n> around to SIZE_MAX - 2 (approx. 18 exabytes), leading to a buffer\n> allocation failure or a crash.\n> \n> Rather than calculating 'path - 1' out of bounds or introducing\n> 'len2', worktree_basename() can simply keep 'name = path' when 'len'\n> is 0. Using an integer index loop 'for (int i = len - 1; 0 <= i;\n> i--)' avoids pointer arithmetic before the start of the buffer\n> entirely, I would think. Or am I missing something?\nInteresting. This function has two types of callers. add_worktree()\ndoes:\n\n\tname = worktree_basename(path, &len);\n\tstrbuf_add(&sb, name, path + len - name);\n\nWhile dwim_branch() and add() do basically:\n\n\tname = worktree_basename(path, &len);\n\tcopy = xstrndup(name, len);\n\nIf path is empty or all separators, len is 0 and name invalid, as noted.\nadd_worktree() breaks, the other callers are mostly fine because\nxstrndup() calls memchr(3) and memcpy(3) with a size of 0 internally\nand thus doesn't dereference the pointer in practice. So patch 2 should\nsuffice to prevent the out of bounds read.\n\nIf path contains one component (\"foo/\"), add_worktree() adds \"foo\" to\nthe strbuf and the others similarly duplicate \"foo\". OK.\n\nIf path contains more components (\"foo/bar/\"), add_worktree() adds\n\"bar\", while the others duplicate \"bar/\", because len is the length of\npath without any trailing separator (7 in this example).\n\nxstrndup(\"bar/\", 7) could read out of bounds because it's calling\nmemchr(3) internally, which doesn't have to stop at the found byte. But\nmore practically, do we really want to keep trailing separators?\n\nworktree_basename() would be harder to misuse if it gave the length of\nthe basename instead. Then add_worktree() wouldn't have to do any\npointer arithmetic and the others wouldn't risk reading out of bounds.\n\nRené\n\n"},{"id":"549429","messageId":"f6b7af1a-29fd-4bec-b819-34b7962180fb@web.de","threadId":"66061","inReplyTo":"ec682d75f3a7848dc36f82cf36bbdff6fd283e2d.1784978348.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] worktree: reject empty string","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2026-08-02T06:26:39Z","receivedAt":"2026-08-02T06:26:47Z","isPatch":true,"body":"On 7/25/26 1:19 PM, Matthias AÃhauer via GitGitGadget wrote:\n> From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= \n> \n> `git worktree add \"\"` errors out with the message `BUG: How come ''\n> becomes empty after sanitization?`, but not due to a bug in the\n> sanitization code. An empty string should remain empty during\n> sanitization. Instead reject the argument as invalid user input,\n> if it's already empty before sanitization.\n> \n> Signed-off-by: Matthias Aßhauer \n> ---\n> builtin/worktree.c | 2 ++\n> 1 file changed, 2 insertions(+)\n> \n> diff --git a/builtin/worktree.c b/builtin/worktree.c\n> index d8188035db..113dbf98d3 100644\n> --- a/builtin/worktree.c\n> +++ b/builtin/worktree.c\n> @@ -496,6 +496,8 @@ static int add_worktree(const char *path, const char *refname,\n> \t\tdie(_(\"invalid reference: %s\"), refname);\n> \n> \tname = worktree_basename(path, &len);\n> +\tif (!len)\n> +\t\tdie(_(\"the empty string is not a valid worktree\"));\n> \tstrbuf_add(&sb, name, path + len - name);\n> \tsanitize_refname_component(sb.buf, &sb_name);\n> \tif (!sb_name.len)\n\nHmm, on my machine, with or without this patch:\n\n $ git worktree add \"\"\n Preparing worktree (new branch '')\n fatal: '' is not a valid branch name\n hint: See 'git help check-ref-format'\n hint: Disable this message with \"git config set advice.refSyntax false\"\n\nand\n\n $ git worktree add /\n Preparing worktree (new branch '')\n fatal: '' is not a valid branch name\n hint: See 'git help check-ref-format'\n hint: Disable this message with \"git config set advice.refSyntax false\"\n\nThis error message is produced by the command 'git branch \"\" HEAD'\nissued using run_command() in add(), just before the the add_worktree()\ncall, which is then skipped.\n\nRené\n\n"},{"id":"549434","messageId":"077f11be-489f-4174-adbc-82a610137a41@web.de","threadId":"66061","inReplyTo":"f6b7af1a-29fd-4bec-b819-34b7962180fb@web.de","subject":"Re: [PATCH 2/2] worktree: reject empty string","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2026-08-02T09:58:05Z","receivedAt":"2026-08-02T09:58:13Z","isPatch":true,"body":"On 8/2/26 8:26 AM, René Scharfe wrote:\n> On 7/25/26 1:19 PM, Matthias AÃhauer via GitGitGadget wrote:\n>> From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= \n>>\n>> `git worktree add \"\"` errors out with the message `BUG: How come ''\n>> becomes empty after sanitization?`, but not due to a bug in the\n>> sanitization code. An empty string should remain empty during\n>> sanitization. Instead reject the argument as invalid user input,\n>> if it's already empty before sanitization.\n>>\n>> Signed-off-by: Matthias Aßhauer \n>> ---\n>> builtin/worktree.c | 2 ++\n>> 1 file changed, 2 insertions(+)\n>>\n>> diff --git a/builtin/worktree.c b/builtin/worktree.c\n>> index d8188035db..113dbf98d3 100644\n>> --- a/builtin/worktree.c\n>> +++ b/builtin/worktree.c\n>> @@ -496,6 +496,8 @@ static int add_worktree(const char *path, const char *refname,\n>> \t\tdie(_(\"invalid reference: %s\"), refname);\n>> \n>> \tname = worktree_basename(path, &len);\n>> +\tif (!len)\n>> +\t\tdie(_(\"the empty string is not a valid worktree\"));\n>> \tstrbuf_add(&sb, name, path + len - name);\n>> \tsanitize_refname_component(sb.buf, &sb_name);\n>> \tif (!sb_name.len)\n> \n> Hmm, on my machine, with or without this patch:\n> \n> $ git worktree add \"\"\n> Preparing worktree (new branch '')\n> fatal: '' is not a valid branch name\n> hint: See 'git help check-ref-format'\n> hint: Disable this message with \"git config set advice.refSyntax false\"\nThis hits the BUG by passing the empty string directly to add_worktree():\n\n $ git worktree add \"\" HEAD\n Preparing worktree (detached HEAD a97fcc37c2)\n BUG: builtin/worktree.c:498: How come '' becomes empty after sanitization?\n\nRené\n\n"},{"id":"550330","messageId":"52ee6501-24ac-402b-b650-92a829030380@web.de","threadId":"66061","inReplyTo":"pull.2187.git.1784978348.gitgitgadget@gmail.com","subject":"[PATCH v1.5] worktree: Fix out of bounds read that causes data loss and reject invalid empty input in worktree add","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2026-08-11T21:34:07Z","receivedAt":"2026-08-11T21:34:18Z","isPatch":true,"body":"From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= \n\n`worktree_basename` tries to read from memory before the passed `path`\nstring, if `path` is empty (or only consists of directory separators).\nThat results in unexpected nonsense data being returned to the caller,\nwhich can lead to issues, such as `git worktree add \"\"` recursively\ndeleting the current working directory, including `.git`.\n\nStop reading out of bounds in these cases to avoid that behaviour.\n\nThis leads to `git worktree add \"\"` consistently exiting with the\nmessage `BUG: How come '' becomes empty after sanitization?`, which is\nstill undesirable, but at least it doesn't result in data loss anymore.\n\nThis fixes https://github.com/git-for-windows/git/issues/6346\n\nSigned-off-by: René Scharfe \n---\nHow about this while we're waiting for a reroll? It implements what the\ncommit message says, nothing more. Follows the style of the first loop.\n\n builtin/worktree.c | 8 +++-----\n 1 file changed, 3 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 654d27c3e1..a770dd5ead 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -303,11 +303,9 @@ static const char *worktree_basename(const char *path, int *olen)\n \twhile (len && is_dir_sep(path[len - 1]))\n \t\tlen--;\n \n-\tfor (name = path + len - 1; name > path; name--)\n-\t\tif (is_dir_sep(*name)) {\n-\t\t\tname++;\n-\t\t\tbreak;\n-\t\t}\n+\tname = path + len;\n+\twhile (name > path && !is_dir_sep(name[-1]))\n+\t\tname--;\n \n \t*olen = len;\n \treturn name;\n-- \n2.55.0\n"},{"id":"550336","messageId":"xmqqwltwz36a.fsf@gitster.g","threadId":"66061","inReplyTo":"52ee6501-24ac-402b-b650-92a829030380@web.de","subject":"Re: [PATCH v1.5] worktree: Fix out of bounds read that causes data loss and reject invalid empty input in worktree add","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-11T22:46:05Z","receivedAt":"2026-08-11T22:46:08Z","isPatch":true,"body":"René Scharfe writes:\n\n> From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= \n>\n> `worktree_basename` tries to read from memory before the passed `path`\n> string, if `path` is empty (or only consists of directory separators).\n> That results in unexpected nonsense data being returned to the caller,\n> which can lead to issues, such as `git worktree add \"\"` recursively\n> deleting the current working directory, including `.git`.\n>\n> Stop reading out of bounds in these cases to avoid that behaviour.\n>\n> This leads to `git worktree add \"\"` consistently exiting with the\n> message `BUG: How come '' becomes empty after sanitization?`, which is\n> still undesirable, but at least it doesn't result in data loss anymore.\n>\n> This fixes https://github.com/git-for-windows/git/issues/6346\n>\n> Signed-off-by: René Scharfe \n> ---\n> How about this while we're waiting for a reroll? It implements what the\n> commit message says, nothing more. Follows the style of the first loop.\n\nThis one I think is obvious and clear. Why not take the authorship\ntoo so that we do not have to worry about DCO?\n\n>\n> builtin/worktree.c | 8 +++-----\n> 1 file changed, 3 insertions(+), 5 deletions(-)\n>\n> diff --git a/builtin/worktree.c b/builtin/worktree.c\n> index 654d27c3e1..a770dd5ead 100644\n> --- a/builtin/worktree.c\n> +++ b/builtin/worktree.c\n> @@ -303,11 +303,9 @@ static const char *worktree_basename(const char *path, int *olen)\n> \twhile (len && is_dir_sep(path[len - 1]))\n> \t\tlen--;\n> \n> -\tfor (name = path + len - 1; name > path; name--)\n> -\t\tif (is_dir_sep(*name)) {\n> -\t\t\tname++;\n> -\t\t\tbreak;\n> -\t\t}\n> +\tname = path + len;\n> +\twhile (name > path && !is_dir_sep(name[-1]))\n> +\t\tname--;\n> \n> \t*olen = len;\n> \treturn name;\n"},{"id":"550673","messageId":"17e8c4e6-9eeb-4c71-9297-d8d5771217d8@web.de","threadId":"66061","inReplyTo":"xmqqwltwz36a.fsf@gitster.g","subject":"Re: [PATCH v1.5] worktree: Fix out of bounds read that causes data loss and reject invalid empty input in worktree add","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2026-08-16T17:51:32Z","receivedAt":"2026-08-16T17:51:49Z","isPatch":true,"body":"On 8/12/26 12:46 AM, Junio C Hamano wrote:\n> René Scharfe writes:\n> \n>> From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= \n>>\n>> `worktree_basename` tries to read from memory before the passed `path`\n>> string, if `path` is empty (or only consists of directory separators).\n>> That results in unexpected nonsense data being returned to the caller,\n>> which can lead to issues, such as `git worktree add \"\"` recursively\n>> deleting the current working directory, including `.git`.\n>>\n>> Stop reading out of bounds in these cases to avoid that behaviour.\n>>\n>> This leads to `git worktree add \"\"` consistently exiting with the\n>> message `BUG: How come '' becomes empty after sanitization?`, which is\n>> still undesirable, but at least it doesn't result in data loss anymore.\n>>\n>> This fixes https://github.com/git-for-windows/git/issues/6346\n>>\n>> Signed-off-by: René Scharfe \n>> ---\n>> How about this while we're waiting for a reroll? It implements what the\n>> commit message says, nothing more. Follows the style of the first loop.\n> \n> This one I think is obvious and clear. Why not take the authorship\n> too so that we do not have to worry about DCO?\n\nThat feels unfair: Matthias did most of the work by identifying the bug\nand removing the premature subtraction from the loop doesn't seem very\noriginal to me. Ultimately my main concern is getting this surprisingly\nimpactful bug fixed in a reasonable amount of time, though..\n\nRené\n\n> \n>>\n>> builtin/worktree.c | 8 +++-----\n>> 1 file changed, 3 insertions(+), 5 deletions(-)\n>>\n>> diff --git a/builtin/worktree.c b/builtin/worktree.c\n>> index 654d27c3e1..a770dd5ead 100644\n>> --- a/builtin/worktree.c\n>> +++ b/builtin/worktree.c\n>> @@ -303,11 +303,9 @@ static const char *worktree_basename(const char *path, int *olen)\n>> \twhile (len && is_dir_sep(path[len - 1]))\n>> \t\tlen--;\n>> \n>> -\tfor (name = path + len - 1; name > path; name--)\n>> -\t\tif (is_dir_sep(*name)) {\n>> -\t\t\tname++;\n>> -\t\t\tbreak;\n>> -\t\t}\n>> +\tname = path + len;\n>> +\twhile (name > path && !is_dir_sep(name[-1]))\n>> +\t\tname--;\n>> \n>> \t*olen = len;\n>> \treturn name;\n\n"},{"id":"550675","messageId":"xmqqik599828.fsf@gitster.g","threadId":"66061","inReplyTo":"17e8c4e6-9eeb-4c71-9297-d8d5771217d8@web.de","subject":"Re: [PATCH v1.5] worktree: Fix out of bounds read that causes data loss and reject invalid empty input in worktree add","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-16T19:30:55Z","receivedAt":"2026-08-16T19:30:58Z","isPatch":true,"body":"René Scharfe writes:\n\n> On 8/12/26 12:46 AM, Junio C Hamano wrote:\n>> René Scharfe writes:\n>> \n>>> From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= \n>>>\n>>> `worktree_basename` tries to read from memory before the passed `path`\n>>> string, if `path` is empty (or only consists of directory separators).\n>>> That results in unexpected nonsense data being returned to the caller,\n>>> which can lead to issues, such as `git worktree add \"\"` recursively\n>>> deleting the current working directory, including `.git`.\n>>>\n>>> Stop reading out of bounds in these cases to avoid that behaviour.\n>>>\n>>> This leads to `git worktree add \"\"` consistently exiting with the\n>>> message `BUG: How come '' becomes empty after sanitization?`, which is\n>>> still undesirable, but at least it doesn't result in data loss anymore.\n>>>\n>>> This fixes https://github.com/git-for-windows/git/issues/6346\n>>>\n>>> Signed-off-by: René Scharfe \n>>> ---\n>>> How about this while we're waiting for a reroll? It implements what the\n>>> commit message says, nothing more. Follows the style of the first loop.\n>> \n>> This one I think is obvious and clear. Why not take the authorship\n>> too so that we do not have to worry about DCO?\n>\n> That feels unfair: Matthias did most of the work by identifying the bug\n> and removing the premature subtraction from the loop doesn't seem very\n> original to me. Ultimately my main concern is getting this surprisingly\n> impactful bug fixed in a reasonable amount of time, though..\n>\n> René\n\nIf we can get Matthias sign this patch off, that would work for me,\ntoo.\n"}]}