From: Taylor Blau <me@ttaylorr.com>
Date: Sat, 16 Mar 2024 01:55:47 GMT
Subject: Re: [PATCH] bugreport.c: fix a crash in `git bugreport` with `--no-suffix` option
Message-ID: <ZfT8I3eoU0dBQMsv@nand.local>
In-Reply-To: <xmqqle6mdsw7.fsf@gitster.g>

On Wed, Mar 13, 2024 at 08:59:52AM -0700, Junio C Hamano wrote:
> "barroit via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Jiamu Sun <barroit@linux.com>
> >
> > executing `git bugreport --no-suffix` led to a segmentation fault
> > due to strbuf_addftime() being called with a NULL option_suffix
> > variable. This occurs because negating the "--[no-]suffix" option
> > causes the parser to set option_suffix to NULL, which is not
> > handled prior to calling strbuf_addftime().
> >
> > Signed-off-by: Jiamu Sun <barroit@linux.com>
> > ---
>
> "git blame" points at 238b439d (bugreport: add tool to generate
> debugging info, 2020-04-16) that is the very beginning of this tool,
> and the bug survived 4f6460df (builtin/bugreport.c: use thread-safe
> localtime_r(), 2020-11-30).  Apparently neither commit considered
> "--suffix=<string>" would invite users to say "--no-suffix" (authors
> of them CC'ed for their input).

I can't speak for 238b439d, but at least in the case of 4f6460df, the
conversion was purely about changing localtime() to localtime_r(), and
nothing more.

The commit message indicates that I was blindly grepping around for
'localtime\(_.\)\?' without looking too much at the surrounding context.

Thanks,
Taylor