Re: Segfault when rebasing with --autosquash

[Jeff King <peff@peff.net>]

On Sat, May 02, 2020 at 09:22:09PM -0400, Paul Ganssle wrote:

> Unfortunately, I have been unable to /deliberately/ create a repository
> that exhibits this behavior using `git absorb`, but last time it
> happened I created a fork of my repo and trimmed out as many commits as
> I could while still exhibiting the behavior, you can find it here, along
> with instructions on how to trigger the bug:
> https://github.com/pganssle-bug-mwes/git_autosquash_bug_mwe

Thanks, this is a good reproduction recipe.

The todo list looks like this:

  pick f4af993 Add sphinx configuration
  pick 1203b9a Add tox environment for building docs
  pick bc28c93 Add minimal documentation for the ZoneInfo module
  pick a5f6347 fixup! Add minimal documentation for the ZoneInfo module
  pick ba18026 fixup! Add sphinx configuration
  pick 04bac75 fixup! Add sphinx configuration
  pick d25a6ae fixup! Add sphinx configuration
  pick 3747813 fixup! Add minimal documentation for the ZoneInfo module
  pick 64ebca8 fixup! Add minimal documentation for the ZoneInfo module
  pick 6c6a5a2 fixup! 3747813a8df9675a7c8b33c4fc665adc52d86b5b

The interesting one is the last one, which is a fixup of another fixup.

Building with ASan, I get this:

  ==1857141==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60400000084c at pc 0x555555c3a37b bp 0x7fffffffbfb0 sp 0x7fffffffbfa8
  WRITE of size 4 at 0x60400000084c thread T0
      #0 0x555555c3a37a in todo_list_rearrange_squash /home/peff/compile/git/sequencer.c:5391
      #1 0x555555c386af in complete_action /home/peff/compile/git/sequencer.c:5196
      #2 0x555555842c2f in do_interactive_rebase builtin/rebase.c:366
      #3 0x55555584320a in run_sequencer_rebase builtin/rebase.c:401
      #4 0x555555849fc5 in run_specific_rebase builtin/rebase.c:949
      #5 0x555555855b29 in cmd_rebase builtin/rebase.c:2071
      #6 0x5555556f23b4 in run_builtin /home/peff/compile/git/git.c:447
      #7 0x5555556f2b06 in handle_builtin /home/peff/compile/git/git.c:672
      #8 0x5555556f3d2d in cmd_main /home/peff/compile/git/git.c:840
      #9 0x555555900565 in main /home/peff/compile/git/common-main.c:52
      #10 0x7ffff7322e0a in __libc_start_main ../csu/libc-start.c:308
      #11 0x5555556ee0d9 in _start (/home/peff/compile/git/git-rebase+0x19a0d9)

The code in question looks like this:

                  if (i2 >= 0) {
                          rearranged = 1;
                          todo_list->items[i].command =
                                  starts_with(subject, "fixup!") ?
                                  TODO_FIXUP : TODO_SQUASH;
                          if (next[i2] < 0)
                                  next[i2] = i;
                          else
                                  next[tail[i2]] = i;
                          tail[i2] = i;

where "i" is the index of the fixup commit and "i2" is the index of the
thing we're fixing up. When we hit that final fixup, our next[i2] will
not be negative. The commit we're fixing is already part of a fixup
chain. But it's not _itself_ being fixed up already, so its tail[i2] is
negative. And thus we'll try to write to next[-1].

I'm not sure if there's a simple logic error here, or a more fundamental
issue with the "next" array having an overloaded meaning. This all comes
from c44a4c650c (rebase -i: rearrange fixup/squash lines using the
rebase--helper, 2017-07-14), so I've added Dscho to the cc.

-Peff