Subject: [RFC PATCH v2 12/12] clean: fix theoretical path corruption

From: Elijah Newren <[email-masked]>
Date: Thu, 5 Sep 2019 08:47:35 -0700

cmd_clean() had the following code structure:

    struct strbuf abs_path = STRBUF_INIT;
    for_each_string_list_item(item, &del_list) {
        strbuf_addstr(&abs_path, prefix);
        strbuf_addstr(&abs_path, item->string);
        PROCESS(&abs_path);
        strbuf_reset(&abs_path);
    }

where I've elided a bunch of unnecessary details and PROCESS(&abs_path)
represents a big chunk of code rather than an actual function call.  One
piece of PROCESS was:

    if (lstat(abs_path.buf, &st))
        continue;

which would cause the strbuf_reset() to be missed -- meaning that the
next path to be handled would have two paths concatenated.  This path
used to use die_errno() instead of continue prior to commit 396049e5fb62
("git-clean: refactor git-clean into two phases", 2013-06-25), but my
understanding of how correct_untracked_entries() works is that it will
prevent both dir/ and dir/file from being in the list to clean so this
should be dead code and the die_errno() should be safe.  But I hesitate
to remove it since I am not certain.  Instead, just fix it to avoid path
corruption in case it is possible to reach this continue statement.

Signed-off-by: Elijah Newren <[email-masked]>
---
 builtin/clean.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index 6030842f3a..ccb6e23f0b 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -1028,8 +1028,10 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 		 * recursive directory removal, so lstat() here could
 		 * fail with ENOENT.
 		 */
-		if (lstat(abs_path.buf, &st))
+		if (lstat(abs_path.buf, &st)) {
+			strbuf_reset(&abs_path);
 			continue;
+		}
 
 		if (S_ISDIR(st.st_mode)) {
 			if (remove_dirs(&abs_path, prefix, rm_flags, dry_run, quiet, &gone))
-- 
2.22.1.11.g45a39ee867