diff mbox series

[v2,07/24] builtin/submodule--helper: fix leaking clone depth parameter

Message ID d3eb0372bd3aeb824d24114ed0e3f01b2fdc6adc.1722499961.git.ps@pks.im (mailing list archive)
State Accepted
Commit 5535b3f3d3eaafa872514af05f021ae6d00f83dc
Headers show
Series Memory leak fixes (pt.3) | expand

Commit Message

Patrick Steinhardt Aug. 1, 2024, 10:42 a.m. UTC
The submodule helper supports a `--depth` parameter for both its "add"
and "clone" subcommands, which in both cases end up being forwarded to
git-clone(1). But while the former subcommand uses an `OPT_INTEGER()` to
parse the depth, the latter uses `OPT_STRING()`. Consequently, it is
possible to pass non-integer input to "--depth" when calling the "clone"
subcommand, where the value will then ultimately cause git-clone(1) to
bail out.

Besides the fact that the parameter verification should happen earlier,
the submodule helper infrastructure also internally tracks the depth via
a string. This requires us to convert the integer in the "add"
subcommand into an allocated string, and this string ultimately leaks.

Refactor the code to consistently track the clone depth as an integer.
This plugs the memory leak, simplifies the code and allows us to use
`OPT_INTEGER()` instead of `OPT_STRING()`, validating the input before
we shell out to git--clone(1).

Original-patch-by: Rubén Justo <rjusto@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/submodule--helper.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
diff mbox series

Patch

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index f1218a1995..863b01627c 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1530,7 +1530,7 @@  struct module_clone_data {
 	const char *path;
 	const char *name;
 	const char *url;
-	const char *depth;
+	int depth;
 	struct list_objects_filter_options *filter_options;
 	unsigned int quiet: 1;
 	unsigned int progress: 1;
@@ -1729,8 +1729,8 @@  static int clone_submodule(const struct module_clone_data *clone_data,
 			strvec_push(&cp.args, "--quiet");
 		if (clone_data->progress)
 			strvec_push(&cp.args, "--progress");
-		if (clone_data->depth && *(clone_data->depth))
-			strvec_pushl(&cp.args, "--depth", clone_data->depth, NULL);
+		if (clone_data->depth > 0)
+			strvec_pushf(&cp.args, "--depth=%d", clone_data->depth);
 		if (reference->nr) {
 			struct string_list_item *item;
 
@@ -1851,8 +1851,7 @@  static int module_clone(int argc, const char **argv, const char *prefix)
 			   N_("reference repository")),
 		OPT_BOOL(0, "dissociate", &dissociate,
 			   N_("use --reference only while cloning")),
-		OPT_STRING(0, "depth", &clone_data.depth,
-			   N_("string"),
+		OPT_INTEGER(0, "depth", &clone_data.depth,
 			   N_("depth for shallow clones")),
 		OPT__QUIET(&quiet, "suppress output for cloning a submodule"),
 		OPT_BOOL(0, "progress", &progress,
@@ -3200,7 +3199,7 @@  static int add_submodule(const struct add_data *add_data)
 		}
 		clone_data.dissociate = add_data->dissociate;
 		if (add_data->depth >= 0)
-			clone_data.depth = xstrfmt("%d", add_data->depth);
+			clone_data.depth = add_data->depth;
 
 		if (clone_submodule(&clone_data, &reference))
 			goto cleanup;