builtin/blame: fix out-of-bounds read with excessive --abbrev

In 6411a0a (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, where as printf(3p) does.

Fix the bug by reverting back to printf(3p) and culling the provided
length to keep it from overflowing when cast to an `int`.

Note that when calling `git blame --abbrev=<N>` with an `N` that is
larger than the maximal OID hex size, there is a subtle side effect that
makes it behave _differently_ than specifying said maximal hex size:
When the command outputs boundary, unblamable or ignored commits' OIDs,
those outputs are prefixed with characters indicating this, and the
`abbrev` value is used to align the information that comes after the
OID, clipping it as needed. Specifying a "too large" abbrev value here
tells Git that yes, we want the full OIDs and don't you worry about
alignment.

Thanks to SHA-256 being _larger_ than the default SHA-1-based OIDs, and
thanks to clipping at `GIT_MAX_HEXSZ`, this change of behavior can only
be observed when running the test in SHA-256 mode.

Nevertheless, this means that we cannot cull at `GIT_MAX_HEXSZ` but at a
slightly larger threshold value.

This patch is a slightly modified version of the one sent as
https://lore.kernel.org/git/20250109-b4-pks-blame-truncate-hash-length-v1-1-9ad4bb09e059@pks.im
to the Git mailing list.

Reported-by: Johannes Schindelin <[email protected]>
Signed-off-by: Patrick Steinhardt <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>

Author: pks-t

builtin/blame.c

@@ -476,6 +476,13 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int
 		size_t length = (opt & OUTPUT_LONG_OBJECT_NAME) ?
 			the_hash_algo->hexsz : (size_t) abbrev;
 
+		/*
+		 * Leave enough space for ^, * and ? indicators (boundary,
+		 * unblamable, ignored).
+		 */
+		if (length > GIT_MAX_HEXSZ + 3)
+			length = GIT_MAX_HEXSZ + 3;
+
 		if (opt & OUTPUT_COLOR_LINE) {
 			if (cnt > 0) {
 				color = repeated_meta_color;
@@ -505,7 +512,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int
 			length--;
 			putchar('?');
 		}
-		fwrite(hex, 1, length, stdout);
+		printf("%.*s", (int)length, hex);
 		if (opt & OUTPUT_ANNOTATE_COMPAT) {
 			const char *name;
 			if (opt & OUTPUT_SHOW_EMAIL)

t/t8002-blame.sh

@@ -126,6 +126,10 @@ 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 9000 --abbrev=9000 HEAD..
+'
+
 test_expect_success '--exclude-promisor-objects does not BUG-crash' '
 	test_must_fail git blame --exclude-promisor-objects one
 '