"Derrick Stolee via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> From: =?UTF-8?q?Kleber=20Tarc=C3=ADsio?= <klebertarcisio@xxxxxxxxxxxx>
>
> There are two reasons that we could return NULL early within
> load_commit_graph_chain():
>
> 1. The file does not exist, so the file pointer is NULL.
> 2. The file exists, but is too small to contain a single hash.
>
> These were grouped together when the function was first written in
> 5c84b3396 (commit-graph: load commit-graph chains, 2019-06-18) in order
> to simplify how the 'chain_name' string is freed. However, the current
> code leaves a narrow window where the file pointer is not closed when
> the file exists, but is rejected for being too small.
>
> Split out these cases separately to ensure we close the file in this
> case.
>
> Signed-off-by: Kleber Tarcísio <klebertarcisio@xxxxxxxxxxxx>
> Signed-off-by: Derrick Stolee <derrickstolee@xxxxxxxxxx>
> ---
> commit-graph: close file before returning NULL
>
> This change was originally submitted to the microsoft/git fork [1].
> Kleber discovered this issue using some automated tool they are working
> on. We recommended that this change be submitted to the core Git group,
> but we have not had any word from the original author in some time.
> Hence, I am submitting it on their behalf.
Makes me wonder if it were a better world if fclose() behaved more
like free() ;-)
Will queue. Thanks.
> commit-graph.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/commit-graph.c b/commit-graph.c
> index 441b36016ba..06107beedcb 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -523,10 +523,13 @@ static struct commit_graph *load_commit_graph_chain(struct repository *r,
> stat_res = stat(chain_name, &st);
> free(chain_name);
>
> - if (!fp ||
> - stat_res ||
> - st.st_size <= the_hash_algo->hexsz)
> + if (!fp)
> return NULL;
> + if (stat_res ||
> + st.st_size <= the_hash_algo->hexsz) {
> + fclose(fp);
> + return NULL;
> + }
>
> count = st.st_size / (the_hash_algo->hexsz + 1);
> CALLOC_ARRAY(oids, count);
>
> base-commit: ab1f2765f78e75ee51dface57e1071b3b7f42b09
