From patchwork Fri Jan 10 11:26:17 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13934298 Received: from fout-a1-smtp.messagingengine.com (fout-a1-smtp.messagingengine.com [103.168.172.144]) (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 7B48A20ADCE for ; Fri, 10 Jan 2025 11:26:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.144 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736508389; cv=none; b=nVSGcStwgESSPi3lGWfqLWvmFCS9LFHyUt2xcn3C0rvSqPvaaMB2bgLuOytzbmqg38D0+nAU3343VYv/w8Sne6k0zxmXMD3uOv+on2GSnD09yenEuKpC2yJz52ULdx+E5yNpIJSMTGguyNQh/GoVhivX8OXvq0G/WWizVusBGng= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736508389; c=relaxed/simple; bh=BMgf+5ZpgptyK37bZux6Jc+dyJdjFExsg5MbjQQ2FjI=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=YRzWHTL4MSz6yE71FVkW7/OLtlSA9/OMkHcvQlYNfBHiqKuGIIyhm1JK4B4aVPGdoSlWLWXoR7ESWDo0Iq/ae63dOrWLpvVLLX7POMQ698O7x/8ejnxWI4UqA1PWwOtkB84e7jLNQKS4rdYDwlK3rgLK7BSmzJrFxObuvCPlyE8= 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=GAI3g2Yh; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=p2Lshk+4; arc=none smtp.client-ip=103.168.172.144 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="GAI3g2Yh"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="p2Lshk+4" Received: from phl-compute-09.internal (phl-compute-09.phl.internal [10.202.2.49]) by mailfout.phl.internal (Postfix) with ESMTP id 78C9A1380141; Fri, 10 Jan 2025 06:26:25 -0500 (EST) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-09.internal (MEProxy); Fri, 10 Jan 2025 06:26:25 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding: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=fm2; t=1736508385; x=1736594785; bh=J3/ayXcJqhIGu0HY/H3gcA9MWZmQXgTOZXMyCbB8udo=; b= GAI3g2YhfPTP+zDifrSRRs9zG7A5TeTH8P/hR5dI81izst6WwxHE3kLUaEMqnKQY UJPG0wGov1E1Wse4e9Por0dVbuzKEaM9TFo6Q7nwT2ADWGWFBVDrURgCvaobyoZT W4XbkhW6RZBTP7kDxMkwAIPz8+si9Qo01ony7Cvt6AjIt22l0OHkdTlqXzcd+e4c xpzebQaf6IzsW/JpQg6JfUVfMpDsL5BeRe50NeuaDglLRQr6VjaezC/uhZwD3MVS /7tDysYtmsMnk2f6TdsCURKg5ukhhrBjp2Eh3FNZelaa1ZswAyG2+C/3ng50CXXQ NEGhqLNGW4c/FIjpTtmL0Q== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :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-sender:x-me-sender:x-sasl-enc; s=fm2; t=1736508385; x= 1736594785; bh=J3/ayXcJqhIGu0HY/H3gcA9MWZmQXgTOZXMyCbB8udo=; b=p 2Lshk+4Nz6k3FnzL39tRwOGlWiege+hMLa8q74trsoziuwSxBRef16FAZTWzSYlk /iYufF0OmBDkZQ5e6OxtpGWotzLPeliAhVctGIuHTONeeQGprj1k89hH7/ywAARI S8MbDYuRwDURF/eXCCifdmKUZOvDHoHhRoBEWKxkCQc0w35maUWSqjrSD2zAS+hL MLk4Sj5RvCz5MB70qU22wRWQmDZMYTw6AU+SPM4KJ9Du0ZYlvBBgOVqZUvHZnPBK vEzIfS54gS2CxCCbrSmaBme4he0YL8sCavLbX080D71hfUWvULafz+5Q0lVAzHAl Xwjev4DtcRo88HbGabC6w== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefuddrudegkedgvdejucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtjeertdertdej necuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrih hmqeenucggtffrrghtthgvrhhnpeffueeiudejvdekheeuvdekfeffiedvueelteekudeh jeetkeegvddugfdtgfeileenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmh grihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeegpdhmohguvgep shhmthhpohhuthdprhgtphhtthhopehgihhtshhtvghrsehpohgsohigrdgtohhmpdhrtg hpthhtohepkhhrihhsthhofhhfvghrhhgruhhgshgsrghkkhesfhgrshhtmhgrihhlrdgt ohhmpdhrtghpthhtohepjhhohhgrnhhnvghsrdhstghhihhnuggvlhhinhesghhmgidrug gvpdhrtghpthhtohepghhithesvhhgvghrrdhkvghrnhgvlhdrohhrgh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 10 Jan 2025 06:26:24 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 096e68a9 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 10 Jan 2025 11:26:20 +0000 (UTC) From: Patrick Steinhardt Date: Fri, 10 Jan 2025 12:26:17 +0100 Subject: [PATCH v3 1/2] builtin/blame: fix out-of-bounds read with excessive `--abbrev` Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250110-b4-pks-blame-truncate-hash-length-v3-1-e61f25b68f30@pks.im> References: <20250110-b4-pks-blame-truncate-hash-length-v3-0-e61f25b68f30@pks.im> In-Reply-To: <20250110-b4-pks-blame-truncate-hash-length-v3-0-e61f25b68f30@pks.im> To: git@vger.kernel.org Cc: Johannes Schindelin , Kristoffer Haugsbakk , Junio C Hamano X-Mailer: b4 0.14.2 In 6411a0a896 (builtin/blame: fix type of `length` variable when emitting object ID, 2024-12-06) we have fixed the type of the `length` variable. In order to avoid a cast from `size_t` to `int` in the call to printf(3p) with the "%.*s" formatter we have converted the code to instead use fwrite(3p), which accepts the length as a `size_t`. It was reported though that this makes us read over the end of the OID array when the provided `--abbrev=` length exceeds the length of the object ID. This is because fwrite(3p) of course doesn't stop when it sees a NUL byte, whereas printf(3p) does. Fix the bug by reverting back to printf(3p) and culling the provided length to `GIT_MAX_HEXSZ` to keep it from overflowing when cast to an `int`. Reported-by: Johannes Schindelin Signed-off-by: Patrick Steinhardt --- builtin/blame.c | 3 ++- t/t8002-blame.sh | 8 ++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/builtin/blame.c b/builtin/blame.c index 867032e4c16878ffd56df8a73162b89ca4bd2694..d7630ac89cb7bd6e9ce5d72c6a98aa433b3b12da 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -505,7 +505,8 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int length--; putchar('?'); } - fwrite(hex, 1, length, stdout); + + printf("%.*s", (int)(length < GIT_MAX_HEXSZ ? length : GIT_MAX_HEXSZ), hex); if (opt & OUTPUT_ANNOTATE_COMPAT) { const char *name; if (opt & OUTPUT_SHOW_EMAIL) diff --git a/t/t8002-blame.sh b/t/t8002-blame.sh index 0147de304b4d104cc7f05ea1f8d68f1a07ceb80d..b3f8b63d2e6744dd434f38fd9f10b56cd432141b 100755 --- a/t/t8002-blame.sh +++ b/t/t8002-blame.sh @@ -126,6 +126,14 @@ test_expect_success '--no-abbrev works like --abbrev with full length' ' check_abbrev $hexsz --no-abbrev ' +test_expect_success 'blame --abbrev gets truncated' ' + check_abbrev $hexsz --abbrev=9000 HEAD +' + +test_expect_success 'blame --abbrev gets truncated with boundary commit' ' + check_abbrev $hexsz --abbrev=9000 ^HEAD +' + test_expect_success '--exclude-promisor-objects does not BUG-crash' ' test_must_fail git blame --exclude-promisor-objects one ' From patchwork Fri Jan 10 11:26:18 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13934297 Received: from fhigh-a5-smtp.messagingengine.com (fhigh-a5-smtp.messagingengine.com [103.168.172.156]) (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 D1CED205ABA for ; Fri, 10 Jan 2025 11:26:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.156 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736508389; cv=none; b=s6C7dWQk3kFikkPDgqEfSwDuoHzk4DHlKj2zFzDoYz0vrdCKp9nlsRPcCridS20gTNCJzaugRxFUgU15OIZjLwNn1xwKkpryrE+zT8u1dIYK+OFWxcdFKUh3In5Voog1gFmwibp0vioBjJrubf92/m8VbM9i0xnsXSYEL0NyXMQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736508389; c=relaxed/simple; bh=rroawE1WrcsSTAHmcRxeqBCxzgiMvn5MeQH/x77lfCU=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=Mg9349STP0O3y4LXW2SZ/MSQBAWNCJJ5AAmc6hd0nePjlNHBGWAMWNkUE14qwYexumZr1/32gc1GAfPZlHsJpQF0t/MN/DqB4wiifAUA0bzssFsNKcDJ9ZYKq3P2DVpsygUoCqpRXorEFf4TVU85xwPO8x5mjnW57LKraT+GLnE= 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=kHsC0IWi; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=Kmfnhx7a; arc=none smtp.client-ip=103.168.172.156 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="kHsC0IWi"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="Kmfnhx7a" Received: from phl-compute-08.internal (phl-compute-08.phl.internal [10.202.2.48]) by mailfhigh.phl.internal (Postfix) with ESMTP id A62431140192; Fri, 10 Jan 2025 06:26:26 -0500 (EST) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-08.internal (MEProxy); Fri, 10 Jan 2025 06:26:26 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding: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=fm2; t=1736508386; x=1736594786; bh=lV981FIK1dv54vGvlofa7UIYAaKPo2Dv+0KrZHo7Anw=; b= kHsC0IWiUIpFXEZymZcy2mr8hlaxBcO2mCCTnH9BdyKT25VNTjJzQgpJByjb54eQ 2Oui2zrp3dfuddLDDrpnnnMYyz+dQ7fWbOGFVVai0WNLW2bH1y3NTIS+QfVy+wD2 KhN2CaJw5/ou/+V+kzCbfUlp1wE31CuAP6R5sgvd7lGAT9P28yhYia7DXNfegvTi 7BcS7NiOw8zSBMpZH9VqCitq+HTibAWfE5CYc5IBfu8zV6VwKDSF3cVojhqzrU/I 66CuYd3DfdNblmKXjaA6tjygaaBLSsqn02IfoEqCnAGyH1Eg6HHaf2Fh7Sq4sphu 5eArzuOt8MweT2YQYhomAQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :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-sender:x-me-sender:x-sasl-enc; s=fm2; t=1736508386; x= 1736594786; bh=lV981FIK1dv54vGvlofa7UIYAaKPo2Dv+0KrZHo7Anw=; b=K mfnhx7afsWDe6Tssza2MdebvOeUiP/0+lVrxiHVRnAFJG3k4IfUrZ5f/p0fwc+3b 2eA7pPi/V05cRisUuXRLUl8JudizfOgq3iZoqzkdbc8xEAAw8FaGZ/p9BIQpMuDJ Ar486TQtitYEjp1+tjx76EbqQy0BYcKM8oB/s2PDuJvsAcqExDPoBkd5lhQDZ1uj AiaLZes2OHrbv55IAR6c8/HhJFR8trRdajukli4FpB65UrpRkrpgA2MQbClRThRP V4/bgdVK3vUrZmdvTKLrC9j5WSf+je1CgpI2xjL0GBhDrROfqvb5kdJQEE5gd21R xMPXkQRpIzVQEMxBizduw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefuddrudegkedgvdejucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtjeertdertdej necuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrih hmqeenucggtffrrghtthgvrhhnpeffueeiudejvdekheeuvdekfeffiedvueelteekudeh jeetkeegvddugfdtgfeileenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmh grihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeegpdhmohguvgep shhmthhpohhuthdprhgtphhtthhopehkrhhishhtohhffhgvrhhhrghughhssggrkhhkse hfrghsthhmrghilhdrtghomhdprhgtphhtthhopehjohhhrghnnhgvshdrshgthhhinhgu vghlihhnsehgmhigrdguvgdprhgtphhtthhopehgihhtsehvghgvrhdrkhgvrhhnvghlrd horhhgpdhrtghpthhtohepghhithhsthgvrhesphhosghogidrtghomh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 10 Jan 2025 06:26:25 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id c8e638b2 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 10 Jan 2025 11:26:21 +0000 (UTC) From: Patrick Steinhardt Date: Fri, 10 Jan 2025 12:26:18 +0100 Subject: [PATCH v3 2/2] builtin/blame: fix out-of-bounds write with blank boundary commits Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250110-b4-pks-blame-truncate-hash-length-v3-2-e61f25b68f30@pks.im> References: <20250110-b4-pks-blame-truncate-hash-length-v3-0-e61f25b68f30@pks.im> In-Reply-To: <20250110-b4-pks-blame-truncate-hash-length-v3-0-e61f25b68f30@pks.im> To: git@vger.kernel.org Cc: Johannes Schindelin , Kristoffer Haugsbakk , Junio C Hamano X-Mailer: b4 0.14.2 When passing the `-b` flag to git-blame(1), then any blamed boundary commits which were marked as uninteresting will not get their actual commit ID printed, but will instead be replaced by a couple of spaces. The flag can lead to an out-of-bounds write as though when combined with `--abbrev=` when the abbreviation length is longer than `GIT_MAX_HEXSZ` as we simply use memset(3p) on that array with the user-provided length directly. The result is most likely that we segfault. An obvious fix would be to cull `length` to `GIT_MAX_HEXSZ` many bytes. But when the underlying object ID is SHA1, and if the abbreviated length exceeds the SHA1 length, it would cause us to print more bytes than desired, and the result would be misaligned. Instead, fix the bug by computing the length via strlen(3p). This makes us write as many bytes as the formatted object ID requires and thus effectively limits the length of what we may end up printing to the length of its hash. If `--abbrev=` asks us to abbreviate to something shorter than the full length of the underlying hash function it would be handled by the call to printf(3p) correctly. Signed-off-by: Patrick Steinhardt --- builtin/blame.c | 6 +++--- t/t8002-blame.sh | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/builtin/blame.c b/builtin/blame.c index d7630ac89cb7bd6e9ce5d72c6a98aa433b3b12da..7555c445abe7ca2fa54670ac8fee1d95a6dbafe3 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -489,9 +489,9 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int fputs(color, stdout); if (suspect->commit->object.flags & UNINTERESTING) { - if (blank_boundary) - memset(hex, ' ', length); - else if (!(opt & OUTPUT_ANNOTATE_COMPAT)) { + if (blank_boundary) { + memset(hex, ' ', strlen(hex)); + } else if (!(opt & OUTPUT_ANNOTATE_COMPAT)) { length--; putchar('^'); } diff --git a/t/t8002-blame.sh b/t/t8002-blame.sh index b3f8b63d2e6744dd434f38fd9f10b56cd432141b..1ad039e1234828ca8779ad76147bfa7fe14c5a2e 100755 --- a/t/t8002-blame.sh +++ b/t/t8002-blame.sh @@ -134,6 +134,24 @@ test_expect_success 'blame --abbrev gets truncated with boundary commit' ' check_abbrev $hexsz --abbrev=9000 ^HEAD ' +test_expect_success 'blame --abbrev -b truncates the blank boundary' ' + # Note that `--abbrev=` always gets incremented by 1, which is why we + # expect 11 leading spaces and not 10. + cat >expect <<-EOF && + $(printf "%0.s " $(test_seq 11)) ( 2005-04-07 15:45:13 -0700 1) abbrev + EOF + git blame -b --abbrev=10 ^HEAD -- abbrev.t >actual && + test_cmp expect actual +' + +test_expect_success 'blame with excessive --abbrev and -b culls to hash length' ' + cat >expect <<-EOF && + $(printf "%0.s " $(test_seq $hexsz)) ( 2005-04-07 15:45:13 -0700 1) abbrev + EOF + git blame -b --abbrev=9000 ^HEAD -- abbrev.t >actual && + test_cmp expect actual +' + test_expect_success '--exclude-promisor-objects does not BUG-crash' ' test_must_fail git blame --exclude-promisor-objects one '