@matheusbsbs / matheus-bagliano / commits / 5535b3f3d3

builtin/submodule--helper: fix leaking clone depth parameter

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Aug 1, 2024 at 12:42 UTC 5535b3f3d3eaafa872514af05f021ae6d00f83dc

1 file changed +5 -6

builtin/submodule--helper.c

+5 -6

@@ -1530,7 +1530,7 @@ struct module_clone_data {

1530 const char *path;

1531 const char *name;

1532 const char *url;

1533 - const char *depth;

1533 + int depth;

1534 struct list_objects_filter_options *filter_options;

1535 unsigned int quiet: 1;

1536 unsigned int progress: 1;

@@ -1729,8 +1729,8 @@ static int clone_submodule(const struct module_clone_data *clone_data,

1729 strvec_push(&cp.args, "--quiet");

1730 if (clone_data->progress)

1731 strvec_push(&cp.args, "--progress");

1732 - if (clone_data->depth && *(clone_data->depth))

1733 - strvec_pushl(&cp.args, "--depth", clone_data->depth, NULL);

1732 + if (clone_data->depth > 0)

1733 + strvec_pushf(&cp.args, "--depth=%d", clone_data->depth);

1734 if (reference->nr) {

1735 struct string_list_item *item;

1736

@@ -1851,8 +1851,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)

1851 N_("reference repository")),

1852 OPT_BOOL(0, "dissociate", &dissociate,

1853 N_("use --reference only while cloning")),

1854 - OPT_STRING(0, "depth", &clone_data.depth,

1855 - N_("string"),

1854 + OPT_INTEGER(0, "depth", &clone_data.depth,

1855 N_("depth for shallow clones")),

1856 OPT__QUIET(&quiet, "suppress output for cloning a submodule"),

1857 OPT_BOOL(0, "progress", &progress,

@@ -3200,7 +3199,7 @@ static int add_submodule(const struct add_data *add_data)

3199 }

3200 clone_data.dissociate = add_data->dissociate;

3201 if (add_data->depth >= 0)

3203 - clone_data.depth = xstrfmt("%d", add_data->depth);

3202 + clone_data.depth = add_data->depth;

3203

3204 if (clone_submodule(&clone_data, &reference))

3205 goto cleanup;