@matheusbsbs / matheus-bagliano / commits / d1b44bb764

finalize_object_file(): check for name collision before renaming

We prefer link()/unlink() to rename() for object files, with the idea that we should prefer the data that is already on disk to what is incoming. But we may fall back to rename() if the user has configured us to do so, or if the filesystem seems not to support cross-directory links. This loses the "prefer what is on disk" property. We can mitigate this somewhat by trying to stat() the destination filename before doing the rename. This is racy, since the object could be created between the stat() and rename() calls. But in practice it is expanding the definition of "what is already on disk" to be the point that the function is called. That is enough to deal with any potential attacks where an attacker is trying to collide hashes with what's already in the repository. Co-authored-by: Jeff King <peff@peff.net> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Taylor Blau committed Sep 26, 2024 at 11:22 UTC d1b44bb76442431eca44279da32deed249dcf213

1 file changed +6 -2

object-file.c

+6 -2

@@ -1904,6 +1904,7 @@ static void write_object_file_prepare_literally(const struct git_hash_algo *algo

1904 */

1905 int finalize_object_file(const char *tmpfile, const char *filename)

1906 {

1907 + struct stat st;

1908 int ret = 0;

1909

1910 if (object_creation_mode == OBJECT_CREATION_USES_RENAMES)

@@ -1924,9 +1925,12 @@ int finalize_object_file(const char *tmpfile, const char *filename)

1925 */

1926 if (ret && ret != EEXIST) {

1927 try_rename:

1927 - if (!rename(tmpfile, filename))

1928 + if (!stat(filename, &st))

1929 + ret = EEXIST;

1930 + else if (!rename(tmpfile, filename))

1931 goto out;

1929 - ret = errno;

1932 + else

1933 + ret = errno;

1934 }

1935 unlink_or_warn(tmpfile);

1936 if (ret) {