Skip to content

Commit 1fbb8d7

Browse files
pks-tgitster
authored andcommitted
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, 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 <Johannes.Schindelin@gmx.de> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent e03d2a9 commit 1fbb8d7

File tree

2 files changed

+10
-1
lines changed

2 files changed

+10
-1
lines changed

builtin/blame.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -505,7 +505,8 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int
505505
length--;
506506
putchar('?');
507507
}
508-
fwrite(hex, 1, length, stdout);
508+
509+
printf("%.*s", (int)(length < GIT_MAX_HEXSZ ? length : GIT_MAX_HEXSZ), hex);
509510
if (opt & OUTPUT_ANNOTATE_COMPAT) {
510511
const char *name;
511512
if (opt & OUTPUT_SHOW_EMAIL)

t/t8002-blame.sh

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,14 @@ test_expect_success '--no-abbrev works like --abbrev with full length' '
126126
check_abbrev $hexsz --no-abbrev
127127
'
128128

129+
test_expect_success 'blame --abbrev gets truncated' '
130+
check_abbrev $hexsz --abbrev=9000 HEAD
131+
'
132+
133+
test_expect_success 'blame --abbrev gets truncated with boundary commit' '
134+
check_abbrev $hexsz --abbrev=9000 ^HEAD
135+
'
136+
129137
test_expect_success '--exclude-promisor-objects does not BUG-crash' '
130138
test_must_fail git blame --exclude-promisor-objects one
131139
'

0 commit comments

Comments
 (0)