Taylor Blau <me@xxxxxxxxxxxx> writes:
> ... But in practice it is
> expanding the definition of "what is already on disk" to be the point
> that the function is called.
Yeah, it is a reasonable argument for this additional protection.
It does not make things worse. All it takes is for the attacker to
come a bit earlier to defeat the link/unlink dance, so doing it "the
right way" does not make it fundamentally safer.
I hope all TOCTOU races can be explained away this way ;-).
> Co-authored-by: Jeff King <peff@xxxxxxxx>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
> ---
> object-file.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/object-file.c b/object-file.c
> index 968da27cd41..38407f468a9 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -1937,6 +1937,7 @@ static void write_object_file_prepare_literally(const struct git_hash_algo *algo
> */
> int finalize_object_file(const char *tmpfile, const char *filename)
> {
> + struct stat st;
> int ret = 0;
>
> if (object_creation_mode == OBJECT_CREATION_USES_RENAMES)
> @@ -1957,9 +1958,12 @@ int finalize_object_file(const char *tmpfile, const char *filename)
> */
> if (ret && ret != EEXIST) {
> try_rename:
> - if (!rename(tmpfile, filename))
> + if (!stat(filename, &st))
> + ret = EEXIST;
> + else if (!rename(tmpfile, filename))
> goto out;
> - ret = errno;
> + else
> + ret = errno;
> }
> unlink_or_warn(tmpfile);
> if (ret) {
