From d8d2eb7d6b5e48c2bcb0e71a770f8a05375ac03e Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Mon, 11 Oct 2010 17:41:16 +0200 Subject: [PATCH 1/1] mailmap: fix use of freed memory On an x86_64 system (F13-based), I ran these commands in an empty directory: git init printf '%s\n' \ ' ' \ 'John ' > .mailmap git shortlog < /dev/null Here's the result: (reading log message from standard input) *** glibc detected *** git: free(): invalid pointer: 0x0000000000f53730 *** ======= Backtrace: ========= /lib64/libc.so.6[0x31ba875676] git[0x48c2a5] git[0x4b9858] ... zsh: abort (core dumped) git shortlog What happened? Some .mailmap entry is of the form, while a subsequent one looks like "User Name , and the two email addresses on the right are not identical but are "equal" when using a case-insensitive comparator. Then, when add_mapping is processing the latter line, new_email is NULL and we free me->email, yet do not replace it with a new strdup'd string. Thus, when later we attempt to use the buffer behind that ->email pointer, we reference freed memory. The solution is to free ->email and ->name only if we're about to replace them. [jc: squashed in the tests from Jonathan] Signed-off-by: Jim Meyering Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- mailmap.c | 10 ++++++---- t/t4203-mailmap.sh | 41 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/mailmap.c b/mailmap.c index f80b70129..02fcfde0b 100644 --- a/mailmap.c +++ b/mailmap.c @@ -79,12 +79,14 @@ static void add_mapping(struct string_list *map, if (old_name == NULL) { debug_mm("mailmap: adding (simple) entry for %s at index %d\n", old_email, index); /* Replace current name and new email for simple entry */ - free(me->name); - free(me->email); - if (new_name) + if (new_name) { + free(me->name); me->name = xstrdup(new_name); - if (new_email) + } + if (new_email) { + free(me->email); me->email = xstrdup(new_email); + } } else { struct mailmap_info *mi = xmalloc(sizeof(struct mailmap_info)); debug_mm("mailmap: adding (complex) entry for %s at index %d\n", old_email, index); diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh index 9a7d1b446..f4f82c023 100755 --- a/t/t4203-mailmap.sh +++ b/t/t4203-mailmap.sh @@ -11,6 +11,7 @@ test_expect_success setup ' git commit -m initial && echo two >>one && git add one && + test_tick && git commit --author "nick1 " -m second ' @@ -54,7 +55,7 @@ Repo Guy (1): EOF test_expect_success 'mailmap.file set' ' - mkdir internal_mailmap && + mkdir -p internal_mailmap && echo "Internal Guy " > internal_mailmap/.mailmap && git config mailmap.file internal_mailmap/.mailmap && git shortlog HEAD >actual && @@ -92,6 +93,40 @@ test_expect_success 'mailmap.file non-existant' ' test_cmp expect actual ' +cat >expect <<\EOF +Internal Guy (1): + second + +Repo Guy (1): + initial + +EOF + +test_expect_success 'name entry after email entry' ' + mkdir -p internal_mailmap && + echo " " >internal_mailmap/.mailmap && + echo "Internal Guy " >>internal_mailmap/.mailmap && + git shortlog >actual && + test_cmp expect actual +' + +cat >expect <<\EOF +Internal Guy (1): + second + +Repo Guy (1): + initial + +EOF + +test_expect_success 'name entry after email entry, case-insensitive' ' + mkdir -p internal_mailmap && + echo " " >internal_mailmap/.mailmap && + echo "Internal Guy " >>internal_mailmap/.mailmap && + git shortlog >actual && + test_cmp expect actual +' + cat >expect <<\EOF A U Thor (1): initial @@ -101,7 +136,7 @@ nick1 (1): EOF test_expect_success 'No mailmap files, but configured' ' - rm .mailmap && + rm -f .mailmap internal_mailmap/.mailmap && git shortlog HEAD >actual && test_cmp expect actual ' @@ -153,7 +188,7 @@ test_expect_success 'Shortlog output (complex mapping)' ' test_tick && git commit --author "CTO " -m seventh && - mkdir internal_mailmap && + mkdir -p internal_mailmap && echo "Committed " > internal_mailmap/.mailmap && echo " " >> internal_mailmap/.mailmap && echo "Some Dude nick1 " >> internal_mailmap/.mailmap && -- 2.30.2