Skip to content

Commit

Permalink
builtin/blame: fix out-of-bounds read with excessive --abbrev
Browse files Browse the repository at this point in the history
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 <[email protected]>
Signed-off-by: Patrick Steinhardt <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
  • Loading branch information
pks-t authored and gitster committed Jan 10, 2025
1 parent e03d2a9 commit 1fbb8d7
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 1 deletion.
3 changes: 2 additions & 1 deletion builtin/blame.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 8 additions & 0 deletions t/t8002-blame.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
'
Expand Down

0 comments on commit 1fbb8d7

Please sign in to comment.