[PATCH/RFC 4/9] add sanity length check to format_person_part

[Will Palmer <wmpalmer@gmail.com>]

previously we were relying on the length check from ident.c, ie: relying
on having been given a good object to parse in the first place. If the
object should have triggered the "Impossibly long personal identifier"
check at commit-time, for example, it would have resulted in an overflow
here.

This is another one which, due to the input constraints, would not have
been a real-world problem for regular usage. I was able to cause an
overflow by viewing the log of a commit with an impossibly-long Author,
created via hash-object. This is admittedly a pretty far-fetched
scenario, though it could potentially be considered a security issue.

In any case, the lack of it rubbed me the wrong way when I was
refactoring this section, and it is trivial to add the sanity check.

Signed-off-by: Will Palmer <wmpalmer@gmail.com>
---
 pretty.c |   20 ++++++++++++++------
 1 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/pretty.c b/pretty.c
index d5a724f..8a288f0 100644
--- a/pretty.c
+++ b/pretty.c
@@ -449,6 +449,7 @@ static size_t format_person_part(struct strbuf *sb, char part,
 	unsigned long date = 0;
 	char *ep;
 	const char *name_start, *name_end, *mail_start, *mail_end, *msg_end = msg+len;
+	size_t name_len, mail_len;
 	char person_name[1024];
 	char person_mail[1024];
 
@@ -469,29 +470,36 @@ static size_t format_person_part(struct strbuf *sb, char part,
 	name_end = msg+end;
 	while (name_end > name_start && isspace(*(name_end-1)))
 		name_end--;
+	name_len = name_end-name_start;
+	if (name_len >= sizeof(person_name))
+		goto skip;
 	mail_start = msg+end+1;
 	mail_end = mail_start;
 	while (mail_end < msg_end && *mail_end != '>')
 		mail_end++;
+	mail_len = mail_end-mail_start;
+	if (mail_len >= sizeof(person_mail))
+		goto skip;
 	if (mail_end == msg_end)
 		goto skip;
 	end = mail_end-msg;
 
 	if (part == 'N' || part == 'E') { /* mailmap lookup */
-		strlcpy(person_name, name_start, name_end-name_start+1);
-		strlcpy(person_mail, mail_start, mail_end-mail_start+1);
+		/* copy up to, and including, the end delimiter */
+		strlcpy(person_name, name_start, name_len+1);
+		strlcpy(person_mail, mail_start, mail_len+1);
 		mailmap_name(person_mail, sizeof(person_mail), person_name, sizeof(person_name));
 		name_start = person_name;
-		name_end = name_start + strlen(person_name);
+		name_len = strlen(person_name);
 		mail_start = person_mail;
-		mail_end = mail_start +  strlen(person_mail);
+		mail_len = strlen(person_mail);
 	}
 	if (part == 'n' || part == 'N') {	/* name */
-		strbuf_add(sb, name_start, name_end-name_start);
+		strbuf_add(sb, name_start, name_len);
 		return placeholder_len;
 	}
 	if (part == 'e' || part == 'E') {	/* email */
-		strbuf_add(sb, mail_start, mail_end-mail_start);
+		strbuf_add(sb, mail_start, mail_len);
 		return placeholder_len;
 	}
 
-- 
1.7.4.2