Re: [PATCH v2] builtin/blame: fix out-of-bounds read with excessive `--abbrev`

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jan 9, 2025, 13:49 UTC
Message-ID: <2db1688e-bf2b-8f53-82d0-11c453a64cd8@gmx.de>
In-Reply-To: <Z3_Dt5SpG6qE5_9V@ArchLinux>

Hi Jialuo,

On Thu, 9 Jan 2025, shejialuo wrote:

Show 23 quoted lines

> On Thu, Jan 09, 2025 at 12:48:22PM +0100, Patrick Steinhardt wrote:
>
> > +		printf("%.*s", (int)length, 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..7cf6e0253a5bbd4d6e438e627dc18b47eac4df66 100755
> > --- a/t/t8002-blame.sh
> > +++ b/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 $hexsz --abbrev=9000 HEAD
> > +'
> > +
>
> By the way, I feel this usage is a little strange as the user side. When
> I received the report mail from Johannes today morning, I feel a little
> funny that we allow the value of the `--abrrev` option exceeds the
> `GIT_MAX_HEXSZ` in the first place.

See the explanation I provided in https://lore.kernel.org/git/c439fcaf-11af-7862-9c3c-18dc0842b57d@gmx.de/: When calling `git blame --abbrev=40 HEAD.. -- <file>` (in a SHA-1-based repository), the OIDs are prefixed with a `^` and then the last hex digit will be cut. The reason? Git wants to align the text after the OID.

When calling it with `--abbrev=41`, the full OID is shown.

Ciao, Johannes