Skip to content

Commit

Permalink
pretty: avoid buffer overflow in format_person_part
Browse files Browse the repository at this point in the history
When we parse the name and email from a commit to
pretty-print them, we usually can just put the result
directly into our strbuf result. However, if we are going to
use the mailmap, then we must first copy them into a
NUL-terminated buffer to feed to the mailmap machinery.

We did so by using strlcpy into a static buffer, but we used
it wrong. We fed it the length of the substring we wanted to
copy, but never checked that that length was less than the
size of the destination buffer.

The simplest fix is to just use snprintf to copy the
substring properly while still respecting the destination
buffer's size. It might seem like replacing the static
buffer with a strbuf would help, but we need to feed a
static buffer to the mailmap machinery anyway, so there's
not much benefit to handling arbitrary sizes.

A more ideal solution would be for mailmap to grow an
interface that:

  1. Takes a pointer and length combination, instead of
     assuming a NUL-terminated string.

  2. Returns a pointer to the mailmap's allocated string,
     rather than copying it into the buffer.

Then we could avoid the need for an extra buffer entirely.
However, doing this would involve a lot of refactoring of
mailmap and of string_list (which mailmap uses to store the
map itself). For now, let's do the simplest thing to fix the
bug.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Jeff King authored and Junio C Hamano committed May 22, 2012
1 parent 4b340cf commit c9b4e9e
Showing 1 changed file with 4 additions and 2 deletions.
6 changes: 4 additions & 2 deletions pretty.c
Original file line number Diff line number Diff line change
@@ -547,8 +547,10 @@ static size_t format_person_part(struct strbuf *sb, char part,
mail_end = s.mail_end;

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);
snprintf(person_name, sizeof(person_name), "%.*s",
(int)(name_end - name_start), name_start);
snprintf(person_mail, sizeof(person_mail), "%.*s",
(int)(mail_end - mail_start), mail_start);
mailmap_name(person_mail, sizeof(person_mail), person_name, sizeof(person_name));
name_start = person_name;
name_end = name_start + strlen(person_name);

0 comments on commit c9b4e9e

Please sign in to comment.