[PATCH v2 03/23] git: fix leaking argv when handling builtins

From

Patrick Steinhardt <ps@pks.im>

Date

Sep 26, 2024, 11:46 UTC

Message-ID

<ea3dd851add1eeb5d53063847b73480b5f6245ed.1727351062.git.ps@pks.im>

In-Reply-To

<cover.1727351062.git.ps@pks.im>

  - We leak the `struct strvec` itself because we directly exit after
    calling `run_builtin()`, without bothering about any cleanups.

  - Even if we free'd that vector we'd end up leaking some of its
    strings because `run_builtin()` will modify the array.

Plug both of these leaks.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 git.c           | 22 +++++++++++++++++++---
 t/t0012-help.sh |  1 +
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/git.c b/git.c
index 9a618a2740..3c7fabfda2 100644
--- a/git.c
+++ b/git.c
@@ -711,6 +711,7 @@ static void strip_extension(const char **argv)
 static void handle_builtin(int argc, const char **argv)
 {
 	struct strvec args = STRVEC_INIT;
+	const char **argv_copy = NULL;
 	const char *cmd;
 	struct cmd_struct *builtin;
 
@@ -731,13 +732,28 @@ static void handle_builtin(int argc, const char **argv)
 		}
 
 		argc++;
-		argv = args.v;
+
+		/*
+		 * `run_builtin()` will modify the argv array, so we need to
+		 * create a shallow copy such that we can free all of its
+		 * strings.
+		 */
+		CALLOC_ARRAY(argv_copy, argc + 1);
+		COPY_ARRAY(argv_copy, args.v, argc);
+
+		argv = argv_copy;
 	}
 
 	builtin = get_builtin(cmd);
-	if (builtin)
-		exit(run_builtin(builtin, argc, argv));
+	if (builtin) {
+		int ret = run_builtin(builtin, argc, argv);
+		strvec_clear(&args);
+		free(argv_copy);
+		exit(ret);
+	}
+
 	strvec_clear(&args);
+	free(argv_copy);
 }
 
 static void execv_dashed_external(const char **argv)
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index 1d273d91c2..9eae0d8356 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -2,6 +2,7 @@
 
 test_description='help'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 configure_help () {

-- 
2.46.2.852.g229c0bf0e5.dirty