commit-graph.c: handle commit parsing errors

To write a commit graph chunk, 'write_graph_chunk_data()' takes a list of commits to write and parses each one before writing the necessary data, and continuing on to the next commit in the list. Since the majority of these commits are not parsed ahead of time (an exception is made for the *last* commit in the list, which is parsed early within 'copy_oids_to_commits'), it is possible that calling 'parse_commit_no_graph()' on them may return an error. Failing to catch these errors before de-referencing later calls can result in a undefined memory access and a SIGSEGV. One such example of this is 'get_commit_tree_oid()', which expects a parsed object as its input (in this case, the commit-graph code passes '*list'). If '*list' causes a parse error, the subsequent call will fail. Prevent such an issue by checking the return value of 'parse_commit_no_graph()' to avoid passing an unparsed object to a function which expects a parsed object, thus preventing a segfault. It is worth noting that this fix is really skirting around the issue in object.c's 'parse_object()', which makes it difficult to tell how corrupt an object is without digging into it. Presumably one could change the meaning of 'parse_object' returns, but this would require adjusting each callsite accordingly. Instead of that, add an additional check to the object parsed. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Taylor Blau committed Sep 5, 2019 at 18:04 UTC 16749b8dd2cd2d3ab693ac7ebe110e57cf054005
2 files changed +4 -2
commit-graph.c
+3 -1
@@ -843,7 +843,9 @@ static void write_graph_chunk_data(struct hashfile *f, int hash_len,
843 uint32_t packedDate[2];
844 display_progress(ctx->progress, ++ctx->progress_cnt);
845
846 - parse_commit_no_graph(*list);
846 + if (parse_commit_no_graph(*list))
847 + die(_("unable to parse commit %s"),
848 + oid_to_hex(&(*list)->object.oid));
849 hashwrite(f, get_commit_tree_oid(*list)->hash, hash_len);
850
851 parent = (*list)->parents;
t/t5318-commit-graph.sh
+1 -1
@@ -585,7 +585,7 @@ test_expect_success 'get_commit_tree_in_graph works for non-the_repository' '
585 test_cmp expect actual
586 '
587
588 -test_expect_failure 'corrupt commit-graph write (broken parent)' '
588 +test_expect_success 'corrupt commit-graph write (broken parent)' '
589 rm -rf repo &&
590 git init repo &&
591 (