From 45350aeb11e0d8b5f76a121dfb9fc053af8e6042 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 3 Oct 2022 13:35:02 -0400 Subject: sequencer: detect author name errors in read_author_script() As we parse the author-script file, we check for missing or duplicate lines for GIT_AUTHOR_NAME, etc. But after reading the whole file, our final error conditional checks "date_i" twice and "name_i" not at all. This not only leads to us failing to abort, but we may do an out-of-bounds read on the string_list array. The bug goes back to 442c36bd08 (am: improve author-script error reporting, 2018-10-31), though the code was soon after moved to this spot by bcd33ec25f (add read_author_script() to libgit, 2018-10-31). It was presumably just a typo in 442c36bd08. We'll add test coverage for all the error cases here, though only the GIT_AUTHOR_NAME ones fail (even in a vanilla build they segfault consistently, but certainly with SANITIZE=address). Reported-by: Michael V. Scovetta Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- sequencer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index 5213d16e97..d8ae21d0d2 100644 --- a/sequencer.c +++ b/sequencer.c @@ -872,7 +872,7 @@ int read_author_script(const char *path, char **name, char **email, char **date, error(_("missing 'GIT_AUTHOR_EMAIL'")); if (date_i == -2) error(_("missing 'GIT_AUTHOR_DATE'")); - if (date_i < 0 || email_i < 0 || date_i < 0 || err) + if (name_i < 0 || email_i < 0 || date_i < 0 || err) goto finish; *name = kv.items[name_i].util; *email = kv.items[email_i].util; -- cgit v1.3-5-g9baa