# bugreport - SEGFAULT on 'git show-branch --current --reflog=3' 6 messages from 2022-04-13 to 2022-04-14. Participants: Gregory David, Ævar Arnfjörð Bjarmason. Thread: https://gitlist.dev/t/57722 ## Gregory David, 2022-04-13 08:39 Subject: bugreport - SEGFAULT on 'git show-branch --current --reflog=3' Message-ID: URL: https://gitlist.dev/e/b6113b91-6613-42b5-ca85-1004099b65c4%40p1sec.com ``` Hi, this bugreport is about a SEGFAULT on git binary when running in this repository (for example, as it also segfault on other private repositories): https://framagit.org/groolot-association/osc_looper.git What did you do before the bug happened? (Steps to reproduce your issue) I just wanted to list branches, and so playing with 'git show-branch --current --reflog=3' in the repository https://framagit.org/groolot-association/osc_looper.git What did you expect to happen? (Expected behavior) Not to segfault What happened instead? (Actual behavior) This command fails with a SEGFAULT on version 2.35.1, 2.20.1 and 2.17.1 It also SEGFAULT on branch 'next' of github git repository. What's different between what you expected and what actually happened? Segfault Anything else you want to add: Segfaut happened when '--current' AND '--reflog=3' are provided together Please review the rest of the bug report below. You can delete any lines you don't wish to share. [System Info] git version: git version 2.35.1 cpu: x86_64 no commit associated with this build sizeof-long: 8 sizeof-size_t: 8 shell-path: /bin/sh uname: Linux 5.16.0-4-amd64 #1 SMP PREEMPT Debian 5.16.12-1 (2022-03-08) x86_64 compiler info: gnuc: 11.2 libc info: glibc: 2.33 $SHELL (typically, interactive shell): /usr/bin/zsh Best regards -- Gregory David Security Engineer https://www.p1sec.com ``` ## Ævar Arnfjörð Bjarmason, 2022-04-13 13:57 Subject: Re: bugreport - SEGFAULT on 'git show-branch --current --reflog=3' Message-ID: <220413.864k2xgk51.gmgdl@evledraar.gmail.com> URL: https://gitlist.dev/e/220413.864k2xgk51.gmgdl%40evledraar.gmail.com In-Reply-To: ``` On Wed, Apr 13 2022, Gregory David wrote: > [[PGP Signed Part:Undecided]] > Hi, this bugreport is about a SEGFAULT on git binary when running in > this repository (for example, as it also segfault on other private > repositories): https://framagit.org/groolot-association/osc_looper.git > > > What did you do before the bug happened? (Steps to reproduce your > issue) I just wanted to list branches, and so playing with 'git > show-branch --current --reflog=3' in the repository > https://framagit.org/groolot-association/osc_looper.git > > What did you expect to happen? (Expected behavior) > Not to segfault > > What happened instead? (Actual behavior) > This command fails with a SEGFAULT on version 2.35.1, 2.20.1 and 2.17.1 > It also SEGFAULT on branch 'next' of github git repository. > > What's different between what you expected and what actually happened? > Segfault > > Anything else you want to add: > Segfaut happened when '--current' AND '--reflog=3' are provided together > > Please review the rest of the bug report below. > You can delete any lines you don't wish to share. > > > [System Info] > git version: > git version 2.35.1 > cpu: x86_64 > no commit associated with this build > sizeof-long: 8 > sizeof-size_t: 8 > shell-path: /bin/sh > uname: Linux 5.16.0-4-amd64 #1 SMP PREEMPT Debian 5.16.12-1 (2022-03-08) > x86_64 > compiler info: gnuc: 11.2 > libc info: glibc: 2.33 > $SHELL (typically, interactive shell): /usr/bin/zsh This appears to fix it, but I didn't have time to further validate it, come up with a test etc: diff --git a/builtin/show-branch.c b/builtin/show-branch.c index 330b0553b9d..be3890b2dd6 100644 --- a/builtin/show-branch.c +++ b/builtin/show-branch.c @@ -881,10 +881,9 @@ int cmd_show_branch(int ac, const char **av, const char *prefix) get_color_reset_code(), ref_name[i]); } - if (!reflog) { + if (!reflog || !reflog_msg[i]) /* header lines never need name */ show_one_commit(rev[i], 1); - } else puts(reflog_msg[i]); ``` ## Gregory David, 2022-04-13 15:48 Subject: Re: bugreport - SEGFAULT on 'git show-branch --current --reflog=3' Message-ID: <6d510d78-9b0b-c463-889b-cd3855ccdd1b@p1sec.com> URL: https://gitlist.dev/e/6d510d78-9b0b-c463-889b-cd3855ccdd1b%40p1sec.com In-Reply-To: <220413.864k2xgk51.gmgdl@evledraar.gmail.com> ``` Thanks a lot Aevar! Don't you mean this: diff --git a/builtin/show-branch.c b/builtin/show-branch.c index 330b0553b9..2a5c31729d 100644 --- a/builtin/show-branch.c +++ b/builtin/show-branch.c @@ -881,7 +881,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix) get_color_reset_code(), ref_name[i]); } - if (!reflog) { + if (!reflog || !rev[i]) { /* header lines never need name */ show_one_commit(rev[i], 1); } Best regards. On 13/04/2022 13:57, Ævar Arnfjörð Bjarmason wrote: > > On Wed, Apr 13 2022, Gregory David wrote: > >> [[PGP Signed Part:Undecided]] >> Hi, this bugreport is about a SEGFAULT on git binary when running in >> this repository (for example, as it also segfault on other private >> repositories): https://framagit.org/groolot-association/osc_looper.git >> >> >> What did you do before the bug happened? (Steps to reproduce your >> issue) I just wanted to list branches, and so playing with 'git >> show-branch --current --reflog=3' in the repository >> https://framagit.org/groolot-association/osc_looper.git >> >> What did you expect to happen? (Expected behavior) >> Not to segfault >> >> What happened instead? (Actual behavior) >> This command fails with a SEGFAULT on version 2.35.1, 2.20.1 and 2.17.1 >> It also SEGFAULT on branch 'next' of github git repository. >> >> What's different between what you expected and what actually happened? >> Segfault >> >> Anything else you want to add: >> Segfaut happened when '--current' AND '--reflog=3' are provided together >> >> Please review the rest of the bug report below. >> You can delete any lines you don't wish to share. >> >> >> [System Info] >> git version: >> git version 2.35.1 >> cpu: x86_64 >> no commit associated with this build >> sizeof-long: 8 >> sizeof-size_t: 8 >> shell-path: /bin/sh >> uname: Linux 5.16.0-4-amd64 #1 SMP PREEMPT Debian 5.16.12-1 (2022-03-08) >> x86_64 >> compiler info: gnuc: 11.2 >> libc info: glibc: 2.33 >> $SHELL (typically, interactive shell): /usr/bin/zsh > > This appears to fix it, but I didn't have time to further validate it, > come up with a test etc: > > diff --git a/builtin/show-branch.c b/builtin/show-branch.c > index 330b0553b9d..be3890b2dd6 100644 > --- a/builtin/show-branch.c > +++ b/builtin/show-branch.c > @@ -881,10 +881,9 @@ int cmd_show_branch(int ac, const char **av, const char *prefix) > get_color_reset_code(), ref_name[i]); > } > > - if (!reflog) { > + if (!reflog || !reflog_msg[i]) > /* header lines never need name */ > show_one_commit(rev[i], 1); > - } > else > puts(reflog_msg[i]); > > -- Gregory David Security Engineer https://www.p1sec.com ``` ## Ævar Arnfjörð Bjarmason, 2022-04-13 18:46 Subject: Re: bugreport - SEGFAULT on 'git show-branch --current --reflog=3' Message-ID: <220413.86wnfses7c.gmgdl@evledraar.gmail.com> URL: https://gitlist.dev/e/220413.86wnfses7c.gmgdl%40evledraar.gmail.com In-Reply-To: <6d510d78-9b0b-c463-889b-cd3855ccdd1b@p1sec.com> ``` On Wed, Apr 13 2022, Gregory David wrote: > [[PGP Signed Part:Undecided]] > Thanks a lot Aevar! > > Don't you mean this: > > diff --git a/builtin/show-branch.c b/builtin/show-branch.c > index 330b0553b9..2a5c31729d 100644 > --- a/builtin/show-branch.c > +++ b/builtin/show-branch.c > @@ -881,7 +881,7 @@ int cmd_show_branch(int ac, const char **av, const > char *prefix) > get_color_reset_code(), ref_name[i]); > } > > - if (!reflog) { > + if (!reflog || !rev[i]) { > /* header lines never need name */ > show_one_commit(rev[i], 1); No, but my "suggested" fix was just enough to compile and get past the segfault, i.e. we don't reach the puts(reflog_msg[i]) branch, but of course rev[i] may be NULL too, I didn't check. For this one though: I haven't tried it, but reading show_one_commit() one of the first things we do is dereference the 1st argument, so surely that will segfault too... ``` ## Gregory David, 2022-04-14 07:20 Subject: Re: bugreport - SEGFAULT on 'git show-branch --current --reflog=3' Message-ID: URL: https://gitlist.dev/e/d72ac546-7c0d-6b05-5b8b-0c4b96192135%40p1sec.com In-Reply-To: <220413.86wnfses7c.gmgdl@evledraar.gmail.com> ``` By my side, your first suggestion didn't remove the segfault due to the irrelevant check on 'reflog[i]' that is not use on next line, so unable to segfault. The fact is you pointed the exact place where it happens and my fix of your fix :) remove the segfault. On 13/04/2022 18:46, Ævar Arnfjörð Bjarmason wrote: > > On Wed, Apr 13 2022, Gregory David wrote: > >> [[PGP Signed Part:Undecided]] >> Thanks a lot Aevar! >> >> Don't you mean this: >> >> diff --git a/builtin/show-branch.c b/builtin/show-branch.c >> index 330b0553b9..2a5c31729d 100644 >> --- a/builtin/show-branch.c >> +++ b/builtin/show-branch.c >> @@ -881,7 +881,7 @@ int cmd_show_branch(int ac, const char **av, const >> char *prefix) >> get_color_reset_code(), ref_name[i]); >> } >> >> - if (!reflog) { >> + if (!reflog || !rev[i]) { >> /* header lines never need name */ >> show_one_commit(rev[i], 1); > > No, but my "suggested" fix was just enough to compile and get past the > segfault, i.e. we don't reach the puts(reflog_msg[i]) branch, but of > course rev[i] may be NULL too, I didn't check. > > For this one though: I haven't tried it, but reading show_one_commit() > one of the first things we do is dereference the 1st argument, so surely > that will segfault too... -- Gregory David Security Engineer https://www.p1sec.com ``` ## Gregory David, 2022-04-14 07:40 Subject: Re: bugreport - SEGFAULT on 'git show-branch --current --reflog=3' Message-ID: <1e5d4562-8fa2-431f-fc8b-ecdddff640cd@p1sec.com> URL: https://gitlist.dev/e/1e5d4562-8fa2-431f-fc8b-ecdddff640cd%40p1sec.com In-Reply-To: <220413.86wnfses7c.gmgdl@evledraar.gmail.com> ``` Sorry, my last answer is totally garbage. On 13/04/2022 18:46, Ævar Arnfjörð Bjarmason wrote: > > On Wed, Apr 13 2022, Gregory David wrote: > >> [[PGP Signed Part:Undecided]] >> Thanks a lot Aevar! >> >> Don't you mean this: >> >> diff --git a/builtin/show-branch.c b/builtin/show-branch.c >> index 330b0553b9..2a5c31729d 100644 >> --- a/builtin/show-branch.c >> +++ b/builtin/show-branch.c >> @@ -881,7 +881,7 @@ int cmd_show_branch(int ac, const char **av, const >> char *prefix) >> get_color_reset_code(), ref_name[i]); >> } >> >> - if (!reflog) { >> + if (!reflog || !rev[i]) { >> /* header lines never need name */ >> show_one_commit(rev[i], 1); > > No, but my "suggested" fix was just enough to compile and get past the > segfault, i.e. we don't reach the puts(reflog_msg[i]) branch, but of > course rev[i] may be NULL too, I didn't check. > > For this one though: I haven't tried it, but reading show_one_commit() > one of the first things we do is dereference the 1st argument, so surely > that will segfault too... -- Gregory David Security Engineer https://www.p1sec.com ```