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 b/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 @@ +#!/bin/sh + +test_description='check parsing of badly formed commit objects' +. ./test-lib.sh + +EMPTY_TREE=4b825dc642cb6eb9a060e54bf8d69288fbee4904 + +test_expect_success 'newline right after mergetag header' ' + test_tick && + cat >commit <<-EOF && + tree $EMPTY_TREE + author $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> $GIT_AUTHOR_DATE + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE + 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 +' + +test_done The "git show" fails with: fatal: Out of memory, malloc failed (tried to allocate 18446744073699764927 bytes) 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. -Peff -- 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
