Re: [PATCH] Always check the return value of `repo_read_object_file()`

>
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> There are a couple of places in Git's source code where the return value
> is not checked. As a consequence, they are susceptible to segmentation
> faults.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>     Always check the return value of repo_read_object_file()
>
>     I ran into this today, when I had tried git am -3 to import changes from
>     a repository into a different repository that has the first repository's
>     code vendored in. To make this work, I set
>     GIT_ALTERNATE_OBJECT_DIRECTORIES accordingly for the git am -3 call, but
>     forgot to set it for a subsequent git diff call, which then segfaulted.
>
>     There are still a couple of places left where there are checks but they
>     look dubious to me, as they simply continue as if an empty blob had been
>     read, for example in builtin/tag.c. However, there are checks that avoid
>     segfaults, so I left them alone.
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1650%2Fdscho%2Fsafer-object-reads-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1650/dscho/safer-object-reads-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1650
>
>  bisect.c           |  3 +++
>  builtin/cat-file.c | 10 ++++++++--
>  builtin/grep.c     |  2 ++
>  builtin/notes.c    |  6 ++++--
>  combine-diff.c     |  2 ++
>  rerere.c           |  3 +++
>  6 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/notes.c b/builtin/notes.c
> index e65cae0bcf7..caf20fd5bdd 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -716,9 +716,11 @@ static int append_edit(int argc, const char **argv, const char *prefix)
>                 struct strbuf buf = STRBUF_INIT;
>                 char *prev_buf = repo_read_object_file(the_repository, note, &type, &size);
>
> -               if (prev_buf && size)
> +               if (!prev_buf)
> +                       die(_("unable to read %s"), oid_to_hex(note));

> +               if (size)
>                         strbuf_add(&buf, prev_buf, size);
> -               if (d.buf.len && prev_buf && size)
> +               if (d.buf.len && size)
>                         append_separator(&buf);
>                 strbuf_insert(&d.buf, 0, buf.buf, buf.len);
>
> diff --git a/combine-diff.c b/combine-diff.c
> index db94581f724..d6d6fa16894 100644
> --- a/combine-diff.c
> +++ b/combine-diff.c
> @@ -337,6 +337,8 @@ static char *grab_blob(struct repository *r,
>                 free_filespec(df);
>         } else {
>                 blob = repo_read_object_file(r, oid, &type, size);
> +               if (!blob)
> +                       die(_("unable to read %s"), oid_to_hex(oid));

>                 if (type != OBJ_BLOB)
>                         die("object '%s' is not a blob!", oid_to_hex(oid));
>         }
>
> base-commit: 2a540e432fe5dff3cfa9d3bf7ca56db2ad12ebb9
> --
> gitgitgadget
>