On Wed, Mar 12, 2014 at 05:14:15PM -0400, Jeff King wrote:

> One thing that bugs me about the current code is that the sub-function
> looks one past the end of the length given to it by the caller.
> Switching it to pass "eof - line + 1" resolves that, but is it right?
> The character pointed at by "eof" is either:
>   1. space, if our strchr(line, ' ') found something
>   2. the first character of the next line, if our
>      memchr(line, '\n', eob - line) found something
>   3. Whatever is at eob (end-of-buffer)

This misses a case. We might find no space at all, and eof is NULL. We
never dereference it, so we don't segfault, but it does cause a bogus
giant allocation.

I'm out of time for the day, but here is a test I started on that
demonstrates the failure:

diff --git a/t/t7513-commit-bogus-extra-headers.sh 
index e69de29..834db08 100755
--- a/t/t7513-commit-bogus-extra-headers.sh
+++ b/t/t7513-commit-bogus-extra-headers.sh
@@ -0,0 +1,26 @@
+test_description='check parsing of badly formed commit objects'
+. ./test-lib.sh
+test_expect_success 'newline right after mergetag header' '
+       test_tick &&
+       cat >commit <<-EOF &&
+       tree $EMPTY_TREE
+       mergetag
+       foo
+       EOF
+       commit=$(git hash-object -w -t commit commit) &&
+       cat >expect <<-EOF &&
+       todo...
+       EOF
+       git --no-pager show --show-signature $commit >actual &&
+       test_cmp expect actual

The "git show" fails with:

  fatal: Out of memory, malloc failed (tried to allocate 18446744073699764927 

So I think the whole function could use some refactoring to handle
corner cases better. I'll try to take a look tomorrow, but please feel
free if somebody else wants to take a crack at it.

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to