kernel
/
git
mirror of https://git.kernel.org/pub/scm/git/git.git
1
0
Fork 0
Code Releases Activity

builtin/blame: fix out-of-bounds write with blank boundary commits 

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 <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
maint
Patrick Steinhardt 2025-01-10 12:26:18 +01:00 committed by Junio C Hamano
parent 1fbb8d7ecb
commit e7fb2ca945
2 changed files with 21 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
builtin/blame.c
View File

@ -489,9 +489,9 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int
fputs(color, stdout); fputs(color, stdout);


if (suspect->commit->object.flags & UNINTERESTING) { if (suspect->commit->object.flags & UNINTERESTING) {
if (blank_boundary) if (blank_boundary) {
memset(hex, ' ', length); memset(hex, ' ', strlen(hex));
else if (!(opt & OUTPUT_ANNOTATE_COMPAT)) { } else if (!(opt & OUTPUT_ANNOTATE_COMPAT)) {
length--; length--;
putchar('^'); putchar('^');
} }

18
t/t8002-blame.sh
View File

@ -134,6 +134,24 @@ test_expect_success 'blame --abbrev gets truncated with boundary commit' '
check_abbrev $hexsz --abbrev=9000 ^HEAD 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)) (<author@example.com> 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)) (<author@example.com> 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_expect_success '--exclude-promisor-objects does not BUG-crash' '
test_must_fail git blame --exclude-promisor-objects one test_must_fail git blame --exclude-promisor-objects one
' '