Skip to contentSkip to conversation

Fix segfault for insane ident line

PATCH New Jiang Xin 1 participant 3 messages Jun 15, 2016

lore
Share & tools
lore

Jiang Xin

author
#0 lore raw

PATCH 1/1 Fix segfault for insane ident line

Today when I push a long run repository, git complains:

    $ git push -u github master:doc-tech
    Counting objects: 5575, done.
    Delta compression using up to 8 threads.
    Compressing objects: 100% (2560/2560), done.
    remote: error: object b798e3ffca56af58c2a7728d75027212a558b6d3:invalid author/committer line - missing space before email
    remote: fatal: Error in object
    error: pack-objects died of signal 13
    error: failed to push some refs to ...

When I check the commit, git will segfault:

    $ git show b798e3ffca56af58c2a7728d75027212a558b6d3
    Segmentation fault: 11

But git v1.8.1 works without difficulty, only with a suspect datetime.

    $ gitv1.8.1 log -1 b798e3ffca56af58c2a7728d75027212a558b6d3
    commit b798e3ffca56af58c2a7728d75027212a558b6d3
    Author: leiweiwei<[email protected]>
    Date:   Thu Jan 1 00:00:00 1970 +0000

add blog/typo.mm.

Detail of this commit. (Note that username and email is insane.)

    $ gitv1.8.1 cat-file commit b798e3ffca56af58c2a7728d75027212a558b6d3
    tree a1fcf9257bfbcd75f8c9aa931d1e89dbc60ae308
    parent 566f0a6489316db9c9dd12bfda51ffc75a24a9b0
    author leiweiwei<[email protected]> <leiweiwei<[email protected]>> 1261964093 +0800
    committer leiweiwei<[email protected]> <leiweiwei<[email protected]>> 1261964093 +0800

add blog/typo.mm.

Because git v1.8.1 is good and v1.8.2 is bad, so I run a git bisect on it, then I find this issue was was introduced in v1.8.1-rc1-7-g3c020bd.

commit 3c020bd528d5dc320b82bd787670edfe6695f097
Author: Antoine Pelisse <[email protected]>
Date: Sat Jan 5 22:26:38 2013 +0100

Use split_ident_line to parse author and committer

Currently blame.c::get_acline(), pretty.c::pp_user_info() and shortlog.c::insert_one_record() are parsing author name, email, time and tz themselves.

Use ident.c::split_ident_line() for better code reuse.

Backtrace for this segfault:

    (gdb) bt
    #0  0x00007fff8d8959ae in strtoul_l ()
    #1  0x000000010012da4a in pp_user_info ()
    #2  0x000000010012f811 in pp_header ()
    #3  0x000000010012f546 in pretty_print_commit ()
    #4  0x000000010010dd1f in show_log ()
    #5  0x000000010010e601 in log_tree_commit ()
    #6  0x00000001000565f1 in cmd_log_walk ()
    #7  0x0000000100056c5c in cmd_show ()
    #8  0x0000000100002710 in run_builtin ()
    #9  0x0000000100001717 in handle_internal_command ()
    #10 0x0000000100001f39 in run_argv ()
    #11 0x000000010000155d in main ()

This patch can fix it.

Jiang Xin (1):
Fix segfault for insane ident line

 builtin/blame.c | 14 +++++++++++---
 pretty.c        |  9 +++++++--
 2 files changed, 18 insertions(+), 5 deletions(-)
Signature · 1 line
1.8.2.1.348.gb94490b
author
#1 lore raw

PATCH 1/1 Fix segfault for insane ident line

An insane ident line (generated by old version of git or by hands such as git-hash-object) may cause segfault for git log and others commands.

This is because the split_ident_line function save the result in ident_split, and the returned ident_split.date_begin, ident_split.date_end, ident_split.tz_begin, and ident_split.tz_end maybe NULL pointers for an insame ident line.

This issue was was introduced in v1.8.1-rc1-7-g3c020bd (Use split_ident_line to parse author and committer).

---
 builtin/blame.c | 14 +++++++++++---
 pretty.c        |  9 +++++++--
 2 files changed, 18 insertions(+), 5 deletions(-)
builtin/blame.c +11 -3
index 86100..4b94e 100644
@@ -1375,10 +1375,18 @@ static void get_ac_line(const char *inbuf, const char *what,
 	maillen = ident.mail_end - ident.mail_begin;
 	mailbuf = ident.mail_begin;
 
-	*time = strtoul(ident.date_begin, NULL, 10);
+	if (ident.date_begin != NULL) {
+		*time = strtoul(ident.date_begin, NULL, 10);
+	} else {
+		*time = 0;
+	}
 
-	len = ident.tz_end - ident.tz_begin;
-	strbuf_add(tz, ident.tz_begin, len);
+	if (ident.tz_begin != NULL && ident.tz_end !=NULL) {
+		len = ident.tz_end - ident.tz_begin;
+		strbuf_add(tz, ident.tz_begin, len);
+	} else {
+		strbuf_addstr(tz, "(unknown)");
+	}
 
 	/*
 	 * Now, convert both name and e-mail using mailmap
pretty.c +7 -2
index d3a82..2402f 100644
@@ -438,8 +438,13 @@ void pp_user_info(const struct pretty_print_context *pp,
 	strbuf_add(&name, namebuf, namelen);
 
 	namelen = name.len + mail.len + 3; /* ' ' + '<' + '>' */
-	time = strtoul(ident.date_begin, &date, 10);
-	tz = strtol(date, NULL, 10);
+	if (ident.date_begin != NULL) {
+		time = strtoul(ident.date_begin, &date, 10);
+		tz = strtol(date, NULL, 10);
+	} else {
+		time = 0;
+		tz = 0;
+	}
 
 	if (pp->fmt == CMIT_FMT_EMAIL) {
 		strbuf_addstr(sb, "From: ");
Signature · 1 line
1.8.2.1.348.gb94490b
author
#2 lore raw

Oops, Ignore this patch please, I found they were already fixed by the following commits
of next branch. Maybe next time I should patch something in next branch.

commit 9dbe7c3d7f4424cf0c27c2d4efabf72e58fa76b9

Author: René Scharfe <[email protected]>
Date:   Wed Apr 17 20:33:35 2013 +0200

pretty: handle broken commit headers gracefully

Centralize the parsing of the date and time zone strings in the new helper function show_ident_date() and make sure it checks the pointers provided by split_ident_line() for NULL before use.

commit de5abe9fe91a496d019d62abefe23df9d72fad30

Author: René Scharfe <[email protected]>
Date:   Wed Apr 17 20:33:54 2013 +0200

blame: handle broken commit headers gracefully

split_ident_line() can leave us with the pointers date_begin, date_end, tz_begin and tz_end all set to NULL. Check them before use and supply the same fallback values as in the case of a negative return code from split_ident_line().

The "(unknown)" is not actually shown in the output, though, because it will be converted to a number (zero) eventually.

--
蒋鑫

北京群英汇信息技术有限公司
邮件: [email protected]
网址: http://www.ossxp.com/
博客: http://www.worldhello.net/
微博: http://weibo.com/gotgit/
电话: 18601196889