git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: [PATCH] avoid segfault when reading header of malformed commits
Date: Tue, 22 May 2012 00:52:17 -0400	[thread overview]
Message-ID: <20120522045217.GA23155@sigill.intra.peff.net> (raw)

If a commit object has a header line at the end of the
buffer that is missing its newline (or if it appears so
because the content on the header line contains a stray
NUL), then git will segfault.

Interestingly, this case is explicitly handled and we do
correctly scan the final line for the header we are looking
for. But if we don't find it, we will dereference NULL while
trying to look at the next line.

Git will never generate such a commit, but it's good to be
defensive. We could die() in such a case, but since it's
easy enough to handle it gracefully, let's just issue a
warning and continue (so you could still view such a commit
with "git show", though you might be missing headers after
the NUL).

Signed-off-by: Jeff King <peff@peff.net>
---
I happened to notice this while experimenting with somebody else's
problem and generating a bogus commit with hash-object. I don't know of
any actual implementation which generates this particular bug.

 pretty.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/pretty.c b/pretty.c
index 02a0a2b..dc57e5b 100644
--- a/pretty.c
+++ b/pretty.c
@@ -436,29 +436,32 @@ static void add_merge_info(const struct pretty_print_context *pp,
 
 static char *get_header(const struct commit *commit, const char *key)
 {
 	int key_len = strlen(key);
 	const char *line = commit->buffer;
 
-	for (;;) {
+	while (line) {
 		const char *eol = strchr(line, '\n'), *next;
 
 		if (line == eol)
 			return NULL;
 		if (!eol) {
+			warning("malformed commit (header is missing newline): %s",
+				sha1_to_hex(commit->object.sha1));
 			eol = line + strlen(line);
 			next = NULL;
 		} else
 			next = eol + 1;
 		if (eol - line > key_len &&
 		    !strncmp(line, key, key_len) &&
 		    line[key_len] == ' ') {
 			return xmemdupz(line + key_len + 1, eol - line - key_len - 1);
 		}
 		line = next;
 	}
+	return NULL;
 }
 
 static char *replace_encoding_header(char *buf, const char *encoding)
 {
 	struct strbuf tmp = STRBUF_INIT;
 	size_t start, len;
-- 
1.7.9.7.33.gc430a50

                 reply	other threads:[~2012-05-22  4:52 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120522045217.GA23155@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).