Skip to content

Commit 5535b3f

Browse files
pks-tgitster
authored andcommitted
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 <[email protected]> Signed-off-by: Patrick Steinhardt <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent ac3b143 commit 5535b3f

File tree

1 file changed

+5
-6
lines changed

1 file changed

+5
-6
lines changed

builtin/submodule--helper.c

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1530,7 +1530,7 @@ struct module_clone_data {
15301530
const char *path;
15311531
const char *name;
15321532
const char *url;
1533-
const char *depth;
1533+
int depth;
15341534
struct list_objects_filter_options *filter_options;
15351535
unsigned int quiet: 1;
15361536
unsigned int progress: 1;
@@ -1729,8 +1729,8 @@ static int clone_submodule(const struct module_clone_data *clone_data,
17291729
strvec_push(&cp.args, "--quiet");
17301730
if (clone_data->progress)
17311731
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);
17341734
if (reference->nr) {
17351735
struct string_list_item *item;
17361736

@@ -1851,8 +1851,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
18511851
N_("reference repository")),
18521852
OPT_BOOL(0, "dissociate", &dissociate,
18531853
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,
18561855
N_("depth for shallow clones")),
18571856
OPT__QUIET(&quiet, "suppress output for cloning a submodule"),
18581857
OPT_BOOL(0, "progress", &progress,
@@ -3200,7 +3199,7 @@ static int add_submodule(const struct add_data *add_data)
32003199
}
32013200
clone_data.dissociate = add_data->dissociate;
32023201
if (add_data->depth >= 0)
3203-
clone_data.depth = xstrfmt("%d", add_data->depth);
3202+
clone_data.depth = add_data->depth;
32043203

32053204
if (clone_submodule(&clone_data, &reference))
32063205
goto cleanup;

0 commit comments

Comments
 (0)