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

> On Thu, 9 Jan 2025, Patrick Steinhardt wrote:
>
> > On Thu, Jan 09, 2025 at 11:49:43AM +0100, Johannes Schindelin wrote:
> > > > diff --git a/builtin/blame.c b/builtin/blame.c
> > > > index 867032e4c16878ffd56df8a73162b89ca4bd2694..ad91fe9e97f90625dd2708fbd44bf2dd24a337a6 100644
> > > > --- a/builtin/blame.c
> > > > +++ b/builtin/blame.c
> > > > @@ -475,6 +475,8 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int
> > > >  		char ch;
> > > >  		size_t length = (opt & OUTPUT_LONG_OBJECT_NAME) ?
> > > >  			the_hash_algo->hexsz : (size_t) abbrev;
> > > > +		if (length > GIT_MAX_HEXSZ)
> > > > +			length = GIT_MAX_HEXSZ;
> > >
> > > This causes a subtle change of behavior because there are a couple of
> > > conditional code blocks between this change and the `printf()` call
> > > decrease `length`, i.e. specifying values larger than the maximal hex size
> > > causes potentially-desirable, different behavior (and think about
> > > https://www.hyrumslaw.com/).
> >
> > Alternatively we can move this until after we have done the
> > subtractions. Then we don't have to do weird gymnastics.
>
> Or we can even avoid assiging a maximum altogether:
>
> 		if (length < GIT_MAX_HEXSZ)
> 			printf("%.*s", (int)length, hex);
> 		else
> 			printf("%s", hex);
>
> Or be more consistent with Git's source code style which often prefers
> ternaries, favoring succinctness over readability:
>
> 		printf("%.*s", (int)(length < GIT_MAX_HEXSZ ? length : GIT_MAX_HEXSZ), hex);