| Message ID | ad2e761f4438ac80e947be0f6831fb6467eb4396.1544048946.git.steadmon@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add commit-graph fuzzer and fix buffer overflow | expand |
On 12/5/2018 5:32 PM, Josh Steadmon wrote: > > + if (chunk_lookup + GRAPH_CHUNKLOOKUP_WIDTH > data + graph_size) { > + error(_("chunk lookup table entry missing; graph file may be incomplete")); > + free(graph); > + return NULL; > + } Something I forgot earlier: there are several tests in t5318-commit-graph.sh that use 'git commit-graph verify' to ensure we hit these error conditions on a corrupted commit-graph file. Could you try adding a test there that looks for this error message? Thanks, -Stolee
diff --git a/commit-graph.c b/commit-graph.c index 0755359b1a..fee171a5f3 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -175,10 +175,19 @@ struct commit_graph *parse_commit_graph(void *graph_map, int fd, last_chunk_offset = 8; chunk_lookup = data + 8; for (i = 0; i < graph->num_chunks; i++) { - uint32_t chunk_id = get_be32(chunk_lookup + 0); - uint64_t chunk_offset = get_be64(chunk_lookup + 4); + uint32_t chunk_id; + uint64_t chunk_offset; int chunk_repeated = 0; + if (chunk_lookup + GRAPH_CHUNKLOOKUP_WIDTH > data + graph_size) { + error(_("chunk lookup table entry missing; graph file may be incomplete")); + free(graph); + return NULL; + } + + chunk_id = get_be32(chunk_lookup + 0); + chunk_offset = get_be64(chunk_lookup + 4); + chunk_lookup += GRAPH_CHUNKLOOKUP_WIDTH; if (chunk_offset > graph_size - GIT_MAX_RAWSZ) {
fuzz-commit-graph identified a case where Git will read past the end of a buffer containing a commit graph if the graph's header has an incorrect chunk count. A simple bounds check in parse_commit_graph() prevents this. Signed-off-by: Josh Steadmon <steadmon@google.com> Helped-by: Derrick Stolee <stolee@gmail.com> --- commit-graph.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)