Re: [PATCH v1.5] worktree: Fix out of bounds read that causes data loss and reject invalid empty input in worktree add

> René Scharfe <l.s.r@web.de> writes:
> 
>> 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?

> 
>>
>>  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;