Hello!
I noticed a duplicate condition in the read_author_script function
defined in sequencer.c. I reported this initially to the security
mailing list, but folks there thought it was unlikely to be
interesting from a security perspective, and advised me to report it
here.
In commit 2a7d63a2, sequencer.c:912 looks like:
912 if (name_i == -2)
913 error(_("missing 'GIT_AUTHOR_NAME'"));
914 if (email_i == -2)
915 error(_("missing 'GIT_AUTHOR_EMAIL'"));
916 if (date_i == -2)
917 error(_("missing 'GIT_AUTHOR_DATE'"));
918 if (date_i < 0 || email_i < 0 || date_i < 0 || err) <-- date_i
is referenced here twice
919 goto finish;
I'm fairly sure that line 918 should be:
918 if (name_i < 0 || email_i < 0 || date_i < 0 || err)
I haven't validated this, but I suspect that if
`rebase-merge/author-script` contained two GIT_AUTHOR_NAME fields,
then name_i would be set to -1 (by the error function), but control
wouldn't flow to finish, but instead to line 920 ( *name =
kv.items[name_i].util; ) where it would attempt to access memory
slightly outside of items' memory space.
I haven't been able to actually trigger the bug, but strongly suspect
I'm just not familiar enough with how rebasing works under the covers.
I can dig into this deeper if you'd like, but it looks like an obvious
typo or copy/paste error. Is this something that a maintainer would be
able to take from here?
Thanks so much!!
Mike
