A possible segfault was introduced in c08830de41 (mv: check if
<destination> is a SKIP_WORKTREE_DIR, 2022-08-09).
When running t7001 with SANITIZE=address, problem appears when running:
git mv path1/path2/ .
or
git mv directory ../
or
any <destination> that makes dest_path[0] an empty string.
The add_slash() call could segfault when path argument to it is an empty
string, because it makes an out-of-bounds read to decide if an extra
slash '/' needs to be appended to it.
As add_slash() is used to make sure that a valid pathname to a file in
the given directory can be made by appending a filename after the value
returned from it, if path is an empty string, we want to return it
as-is. The path to a file "F" in the top-level of the working tree
(i.e. path=="") is formed by appending "F" after "" (i.e. path) without
any slash in between.
So, just like the case where a non-empty path already ends with a slash,
return an empty path as-is.
Reported-by: Jeff King <peff@xxxxxxxx>
Helped-by: Jeff King <peff@xxxxxxxx>
Helped-by: Junio C Hamano <gitster@xxxxxxxxx>
Helped-by: Derrick Stolee <derrickstolee@xxxxxxxxxx>
Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@xxxxxxxxx>
---
Range-diff against v2:
1: 1120dc7e6b ! 1: 569e618013 builtin/mv.c: fix possible segfault in add_slash()
@@ Commit message
or
any <destination> that makes dest_path[0] an empty string.
- The add_slash() call could segfault when dest_path[0] is an empty string,
- because it was accessing a null value in such case.
-
- Change add_slash() to check the path argument is a non-empty string
- before accessing its value. If the path is empty, return it as-is.
-
- Explanation:
-
- It's OK for add_slash() to return an empty string as-is. add_slash()
- converts its path argument to the prefix (for "folder1/file1",
- "folder1/" is the prefix we mean here) for the result path. The path
- argument is an empty string _iff_ the result path is analyzed to be at
- the top level (this normalization process is done earlier by
- internal_prefix_pathspec()).
-
- Because the prefix for a top-level path is an empty string, thus
- add_slash() should return an empty path argument as-is, both for
- correctness and avoiding inappropriate memory access.
+ The add_slash() call could segfault when path argument to it is an empty
+ string, because it makes an out-of-bounds read to decide if an extra
+ slash '/' needs to be appended to it.
+
+ As add_slash() is used to make sure that a valid pathname to a file in
+ the given directory can be made by appending a filename after the value
+ returned from it, if path is an empty string, we want to return it
+ as-is. The path to a file "F" in the top-level of the working tree
+ (i.e. path=="") is formed by appending "F" after "" (i.e. path) without
+ any slash in between.
+
+ So, just like the case where a non-empty path already ends with a slash,
+ return an empty path as-is.
Reported-by: Jeff King <peff@xxxxxxxx>
Helped-by: Jeff King <peff@xxxxxxxx>
builtin/mv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/mv.c b/builtin/mv.c
index 2d64c1e80f..3413ad1c9b 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -71,7 +71,7 @@ static const char **internal_prefix_pathspec(const char *prefix,
static const char *add_slash(const char *path)
{
size_t len = strlen(path);
- if (path[len - 1] != '/') {
+ if (len && path[len - 1] != '/') {
char *with_slash = xmalloc(st_add(len, 2));
memcpy(with_slash, path, len);
with_slash[len++] = '/';
base-commit: a6b4b080e4ef65ebbab73e47c0100b5dc12e104c
--
2.37.0
