From: Johannes Schindelin <johannes.schindelin@xxxxxx>
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@xxxxxx>
---
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/bisect.c b/bisect.c
index f1273c787d9..f75e50c3397 100644
--- a/bisect.c
+++ b/bisect.c
@@ -158,6 +158,9 @@ static void show_list(const char *debug, int counted, int nr,
const char *subject_start;
int subject_len;
+ if (!buf)
+ die(_("unable to read %s"), oid_to_hex(&commit->object.oid));
+
fprintf(stderr, "%c%c%c ",
(commit_flags & TREESAME) ? ' ' : 'T',
(commit_flags & UNINTERESTING) ? 'U' : ' ',
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 7d4899348a3..bbf851138ec 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -221,6 +221,10 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
&type,
&size);
const char *target;
+
+ if (!buffer)
+ die(_("unable to read %s"), oid_to_hex(&oid));
+
if (!skip_prefix(buffer, "object ", &target) ||
get_oid_hex(target, &blob_oid))
die("%s not a valid tag", oid_to_hex(&oid));
@@ -416,6 +420,8 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d
contents = repo_read_object_file(the_repository, oid, &type,
&size);
+ if (!contents)
+ die("object %s disappeared", oid_to_hex(oid));
if (use_mailmap) {
size_t s = size;
@@ -423,8 +429,6 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d
size = cast_size_t_to_ulong(s);
}
- if (!contents)
- die("object %s disappeared", oid_to_hex(oid));
if (type != data->type)
die("object %s changed type!?", oid_to_hex(oid));
if (data->info.sizep && size != data->size && !use_mailmap)
@@ -481,6 +485,8 @@ static void batch_object_write(const char *obj_name,
buf = repo_read_object_file(the_repository, &data->oid, &data->type,
&data->size);
+ if (!buf)
+ die(_("unable to read %s"), oid_to_hex(&data->oid));
buf = replace_idents_using_mailmap(buf, &s);
data->size = cast_size_t_to_ulong(s);
diff --git a/builtin/grep.c b/builtin/grep.c
index c8e33f97755..982bcfc4b1d 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -571,6 +571,8 @@ static int grep_cache(struct grep_opt *opt,
data = repo_read_object_file(the_repository, &ce->oid,
&type, &size);
+ if (!data)
+ die(_("unable to read tree %s"), oid_to_hex(&ce->oid));
init_tree_desc(&tree, data, size);
hit |= grep_tree(opt, pathspec, &tree, &name, 0, 0);
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));
}
diff --git a/rerere.c b/rerere.c
index ca7e77ba68c..13c94ded037 100644
--- a/rerere.c
+++ b/rerere.c
@@ -973,6 +973,9 @@ static int handle_cache(struct index_state *istate,
mmfile[i].ptr = repo_read_object_file(the_repository,
&ce->oid, &type,
&size);
+ if (!mmfile[i].ptr)
+ die(_("unable to read %s"),
+ oid_to_hex(&ce->oid));
mmfile[i].size = size;
}
}
base-commit: 2a540e432fe5dff3cfa9d3bf7ca56db2ad12ebb9
--
gitgitgadget
