From patchwork Thu Aug 1 10:42:59 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13750171 Received: from fhigh2-smtp.messagingengine.com (fhigh2-smtp.messagingengine.com [103.168.172.153]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C492D170A17 for ; Thu, 1 Aug 2024 10:43:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.153 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722508985; cv=none; b=MTKoPI4ePTE7b8AqFjreNYVbDXSWI7kl+Aj+P2aJQwUCTWhlPg/1xiM4UW6ybDJN/M9dW0YuLkPOauL46QpOva6NceimXHPy+eHmVPmZwH1sLoTjmtNoKHJ1cgOJ574fTPB0FLJe7ehEdzhyBp9PuSlj9cKijIKWbsIyb2Rxix8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722508985; c=relaxed/simple; bh=+PwiTOy7dbRZrIyY2q2oZWkRUgir+7qP3ftug/YI2/I=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=kL+dTLmE1xiKIzADTMNGyBNhCy5eo0hqrnssN6PMfInYoHDrDLD26jMDvpR+jDZVAuHPsHoNG0bhsMM+GRFDpi+614nH2eviTQAe+hQrseL3NIrVurdKu/ZjcdAeaQa5tIqqdwx6sgOqSq/orAe2H6CSBwMxhHO7OQFo2ZMXNGg= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=UdRlRby7; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=kN/HTev/; arc=none smtp.client-ip=103.168.172.153 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="UdRlRby7"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="kN/HTev/" Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailfhigh.nyi.internal (Postfix) with ESMTP id 1F5C71148363; Thu, 1 Aug 2024 06:43:03 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute5.internal (MEProxy); Thu, 01 Aug 2024 06:43:03 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm3; t=1722508983; x=1722595383; bh=OK4FPYT8UC bFpTMME0E4+XysFllslgBwOZlfBdpzp0E=; b=UdRlRby7fAG0f5fae41SCvURrZ Jr/HFtaY0PCdouk2nmuUWJCMj4a5jnoP0xXmRDjnjQxTZA0Dw7ItctQmAtUnav6+ ey5dKd4HG2N+Ix0l23w0PX1cmz/7ZVp6GnV3LEdQykMv4IUcfX0gWfmQsSB9QY9l ix9Ya+H4CP4yEQpQowpiI7TVTxRQGjBGOmcj8kGAEODkVCSubRoQqKms5VoSpcV/ F6alwJpgt4m85HIHYU8VijSVIdMLaVdlbfM+Bi+G1UDv/pwuR/76BvQxC7HjQGFH QccQfdHDdSS0gihT57DDnCmtZrir0NwIDX0A6+yjDEkoSXyX1YoxU1EheQ2g== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1722508983; x=1722595383; bh=OK4FPYT8UCbFpTMME0E4+XysFlls lgBwOZlfBdpzp0E=; b=kN/HTev/rxSXKCcxbNMuP/M70LM1SJr7gxn11PExNMfq gtbCBcKn3Af0kpAXra1xrhYIdv+hF3Bm7tPzTTrEe5mD4CQDWhy34oqe8TBd/cyg 9dwvvXWlldGpuD9b7JzWHH+SURg0+ZfFflWHRgoQL6ahlYRLVzwApBv/XVmCUpYQ 2aStTrCUrNqbTClWN4HeyE1jXbFa1EC0I+jFYcXWMIbdMaMRU4O9RMOgT9QO5Jsg npisQF+UnLvTxsXFmQt6DPaVE7OFQx30RmNPvOPX3/asI+2N0Ng8bMwAMzcHuydL VX0HCWIALYqIoDGKgdVOMoCLrDvMilvCjy/Du4rXLA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrjeekgdefudcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvvefukfhfgggtuggjsehgtderredttdejnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeetueevhffhudefvdegieeuieelgedthfegfedtueevjeejtdfgjeehudejuedtuden ucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhmpdhnsggprhgtphhtthhopedt X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 1 Aug 2024 06:43:01 -0400 (EDT) Received: by localhost (OpenSMTPD) with ESMTPSA id a8fe73c0 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Thu, 1 Aug 2024 10:41:29 +0000 (UTC) Date: Thu, 1 Aug 2024 12:42:59 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Karthik Nayak , Taylor Blau , Junio C Hamano , =?utf-8?b?UnViw6lu?= Justo Subject: [PATCH v2 07/24] builtin/submodule--helper: fix leaking clone depth parameter Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: 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 Signed-off-by: Patrick Steinhardt --- builtin/submodule--helper.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) 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;