{"thread":{"id":"30585","subject":"[PATCH] avoid segfault when reading header of malformed commits","startedAt":"2012-05-22T04:52:17Z","lastAt":"2012-05-22T04:52:17Z","messageCount":1,"participants":["Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"191871","messageId":"20120522045217.GA23155@sigill.intra.peff.net","threadId":"30585","inReplyTo":null,"subject":"[PATCH] avoid segfault when reading header of malformed commits","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-22T04:52:17Z","receivedAt":"2012-05-22T04:52:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If a commit object has a header line at the end of the\nbuffer that is missing its newline (or if it appears so\nbecause the content on the header line contains a stray\nNUL), then git will segfault.\n\nInterestingly, this case is explicitly handled and we do\ncorrectly scan the final line for the header we are looking\nfor. But if we don't find it, we will dereference NULL while\ntrying to look at the next line.\n\nGit will never generate such a commit, but it's good to be\ndefensive. We could die() in such a case, but since it's\neasy enough to handle it gracefully, let's just issue a\nwarning and continue (so you could still view such a commit\nwith \"git show\", though you might be missing headers after\nthe NUL).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI happened to notice this while experimenting with somebody else's\nproblem and generating a bogus commit with hash-object. I don't know of\nany actual implementation which generates this particular bug.\n\n pretty.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 02a0a2b..dc57e5b 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -436,29 +436,32 @@ static void add_merge_info(const struct pretty_print_context *pp,\n \n static char *get_header(const struct commit *commit, const char *key)\n {\n \tint key_len = strlen(key);\n \tconst char *line = commit->buffer;\n \n-\tfor (;;) {\n+\twhile (line) {\n \t\tconst char *eol = strchr(line, '\\n'), *next;\n \n \t\tif (line == eol)\n \t\t\treturn NULL;\n \t\tif (!eol) {\n+\t\t\twarning(\"malformed commit (header is missing newline): %s\",\n+\t\t\t\tsha1_to_hex(commit->object.sha1));\n \t\t\teol = line + strlen(line);\n \t\t\tnext = NULL;\n \t\t} else\n \t\t\tnext = eol + 1;\n \t\tif (eol - line > key_len &&\n \t\t    !strncmp(line, key, key_len) &&\n \t\t    line[key_len] == ' ') {\n \t\t\treturn xmemdupz(line + key_len + 1, eol - line - key_len - 1);\n \t\t}\n \t\tline = next;\n \t}\n+\treturn NULL;\n }\n \n static char *replace_encoding_header(char *buf, const char *encoding)\n {\n \tstruct strbuf tmp = STRBUF_INIT;\n \tsize_t start, len;\n-- \n1.7.9.7.33.gc430a50\n"}]}