Always check `parse_tree*()`'s return value
Otherwise we may easily run into serious crashes: For example, if we run `init_tree_desc()` directly after a failed `parse_tree()`, we are accessing uninitialized data or trying to dereference `NULL`. Note that the `parse_tree()` function already takes care of showing an error message. The `parse_tree_indirectly()` and `repo_get_commit_tree()` functions do not, therefore those latter call sites need to show a useful error message while the former do not. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Johannes Schindelin committed
Feb 23, 2024 at 08:34 UTC
aa9f618909d30e3f0c7181243e89a81220507e6e
12 files changed
+52
-10
builtin/checkout.c
+16
-3
@@ -707,7 +707,8 @@ static int reset_tree(struct tree *tree, const struct checkout_opts *o,
707
init_checkout_metadata(&opts.meta, info->refname,
708
info->commit ? &info->commit->object.oid : null_oid(),
709
NULL);
710
- parse_tree(tree);
710
+ if (parse_tree(tree) < 0)
711
+ return 128;
712
init_tree_desc(&tree_desc, tree->buffer, tree->size);
713
switch (unpack_trees(1, &tree_desc, &opts)) {
714
case -2:
@@ -786,9 +787,15 @@ static int merge_working_tree(const struct checkout_opts *opts,
787
if (new_branch_info->commit)
788
BUG("'switch --orphan' should never accept a commit as starting point");
789
new_tree = parse_tree_indirect(the_hash_algo->empty_tree);
789
- } else
790
+ if (!new_tree)
791
+ BUG("unable to read empty tree");
792
+ } else {
793
new_tree = repo_get_commit_tree(the_repository,
794
new_branch_info->commit);
795
+ if (!new_tree)
796
+ return error(_("unable to read tree (%s)"),
797
+ oid_to_hex(&new_branch_info->commit->object.oid));
798
+ }
799
if (opts->discard_changes) {
800
ret = reset_tree(new_tree, opts, 1, writeout_error, new_branch_info);
801
if (ret)
@@ -823,7 +830,8 @@ static int merge_working_tree(const struct checkout_opts *opts,
830
oid_to_hex(old_commit_oid));
831
832
init_tree_desc(&trees[0], tree->buffer, tree->size);
826
- parse_tree(new_tree);
833
+ if (parse_tree(new_tree) < 0)
834
+ exit(128);
835
tree = new_tree;
836
init_tree_desc(&trees[1], tree->buffer, tree->size);
837
@@ -1239,10 +1247,15 @@ static void setup_new_branch_info_and_source_tree(
1247
if (!new_branch_info->commit) {
1248
/* not a commit */
1249
*source_tree = parse_tree_indirect(rev);
1250
+ if (!*source_tree)
1251
+ die(_("unable to read tree (%s)"), oid_to_hex(rev));
1252
} else {
1253
parse_commit_or_die(new_branch_info->commit);
1254
*source_tree = repo_get_commit_tree(the_repository,
1255
new_branch_info->commit);
1256
+ if (!*source_tree)
1257
+ die(_("unable to read tree (%s)"),
1258
+ oid_to_hex(&new_branch_info->commit->object.oid));
1259
}
1260
}
1261
builtin/clone.c
+2
-1
@@ -736,7 +736,8 @@ static int checkout(int submodule_progress, int filter_submodules)
736
tree = parse_tree_indirect(&oid);
737
if (!tree)
738
die(_("unable to parse commit %s"), oid_to_hex(&oid));
739
- parse_tree(tree);
739
+ if (parse_tree(tree) < 0)
740
+ exit(128);
741
init_tree_desc(&t, tree->buffer, tree->size);
742
if (unpack_trees(1, &t, &opts) < 0)
743
die(_("unable to checkout working tree"));
builtin/commit.c
+2
-1
@@ -339,7 +339,8 @@ static void create_base_index(const struct commit *current_head)
339
tree = parse_tree_indirect(¤t_head->object.oid);
340
if (!tree)
341
die(_("failed to unpack HEAD tree object"));
342
- parse_tree(tree);
342
+ if (parse_tree(tree) < 0)
343
+ exit(128);
344
init_tree_desc(&t, tree->buffer, tree->size);
345
if (unpack_trees(1, &t, &opts))
346
exit(128); /* We've already reported the error, finish dying */
builtin/merge-tree.c
+6
@@ -447,12 +447,18 @@ static int real_merge(struct merge_tree_options *o,
447
if (repo_get_oid_treeish(the_repository, merge_base, &base_oid))
448
die(_("could not parse as tree '%s'"), merge_base);
449
base_tree = parse_tree_indirect(&base_oid);
450
+ if (!base_tree)
451
+ die(_("unable to read tree (%s)"), oid_to_hex(&base_oid));
452
if (repo_get_oid_treeish(the_repository, branch1, &head_oid))
453
die(_("could not parse as tree '%s'"), branch1);
454
parent1_tree = parse_tree_indirect(&head_oid);
455
+ if (!parent1_tree)
456
+ die(_("unable to read tree (%s)"), oid_to_hex(&head_oid));
457
if (repo_get_oid_treeish(the_repository, branch2, &merge_oid))
458
die(_("could not parse as tree '%s'"), branch2);
459
parent2_tree = parse_tree_indirect(&merge_oid);
460
+ if (!parent2_tree)
461
+ die(_("unable to read tree (%s)"), oid_to_hex(&merge_oid));
462
463
opt.ancestor = merge_base;
464
merge_incore_nonrecursive(&opt, base_tree, parent1_tree, parent2_tree, &result);
builtin/read-tree.c
+2
-1
@@ -263,7 +263,8 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix)
263
cache_tree_free(&the_index.cache_tree);
264
for (i = 0; i < nr_trees; i++) {
265
struct tree *tree = trees[i];
266
- parse_tree(tree);
266
+ if (parse_tree(tree) < 0)
267
+ return 128;
268
init_tree_desc(t+i, tree->buffer, tree->size);
269
}
270
if (unpack_trees(nr_trees, t, &opts))
builtin/reset.c
+4
@@ -119,6 +119,10 @@ static int reset_index(const char *ref, const struct object_id *oid, int reset_t
119
120
if (reset_type == MIXED || reset_type == HARD) {
121
tree = parse_tree_indirect(oid);
122
+ if (!tree) {
123
+ error(_("unable to read tree (%s)"), oid_to_hex(oid));
124
+ goto out;
125
+ }
126
prime_cache_tree(the_repository, the_repository->index, tree);
127
}
128
cache-tree.c
+2
-2
@@ -779,8 +779,8 @@ static void prime_cache_tree_rec(struct repository *r,
779
struct cache_tree_sub *sub;
780
struct tree *subtree = lookup_tree(r, &entry.oid);
781
782
- if (!subtree->object.parsed)
783
- parse_tree(subtree);
782
+ if (!subtree->object.parsed && parse_tree(subtree) < 0)
783
+ exit(128);
784
sub = cache_tree_sub(it, entry.path);
785
sub->cache_tree = cache_tree();
786
merge-ort.c
+3
@@ -4983,6 +4983,9 @@ redo:
4983
4984
if (result->clean >= 0) {
4985
result->tree = parse_tree_indirect(&working_tree_oid);
4986
+ if (!result->tree)
4987
+ die(_("unable to read tree (%s)"),
4988
+ oid_to_hex(&working_tree_oid));
4989
/* existence of conflicted entries implies unclean */
4990
result->clean &= strmap_empty(&opt->priv->conflicted);
4991
}
merge-recursive.c
+2
-1
@@ -410,7 +410,8 @@ static inline int merge_detect_rename(struct merge_options *opt)
410
411
static void init_tree_desc_from_tree(struct tree_desc *desc, struct tree *tree)
412
{
413
- parse_tree(tree);
413
+ if (parse_tree(tree) < 0)
414
+ exit(128);
415
init_tree_desc(desc, tree->buffer, tree->size);
416
}
417
merge.c
+4
-1
@@ -80,7 +80,10 @@ int checkout_fast_forward(struct repository *r,
80
return -1;
81
}
82
for (i = 0; i < nr_trees; i++) {
83
- parse_tree(trees[i]);
83
+ if (parse_tree(trees[i]) < 0) {
84
+ rollback_lock_file(&lock_file);
85
+ return -1;
86
+ }
87
init_tree_desc(t+i, trees[i]->buffer, trees[i]->size);
88
}
89
reset.c
+5
@@ -158,6 +158,11 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
158
}
159
160
tree = parse_tree_indirect(oid);
161
+ if (!tree) {
162
+ ret = error(_("unable to read tree (%s)"), oid_to_hex(oid));
163
+ goto leave_reset_head;
164
+ }
165
+
166
prime_cache_tree(r, r->index, tree);
167
168
if (write_locked_index(r->index, &lock, COMMIT_LOCK) < 0) {
sequencer.c
+4
@@ -715,6 +715,8 @@ static int do_recursive_merge(struct repository *r,
715
o.show_rename_progress = 1;
716
717
head_tree = parse_tree_indirect(head);
718
+ if (!head_tree)
719
+ return error(_("unable to read tree (%s)"), oid_to_hex(head));
720
next_tree = next ? repo_get_commit_tree(r, next) : empty_tree(r);
721
base_tree = base ? repo_get_commit_tree(r, base) : empty_tree(r);
722
@@ -3887,6 +3889,8 @@ static int do_reset(struct repository *r,
3889
}
3890
3891
tree = parse_tree_indirect(&oid);
3892
+ if (!tree)
3893
+ return error(_("unable to read tree (%s)"), oid_to_hex(&oid));
3894
prime_cache_tree(r, r->index, tree);
3895
3896
if (write_locked_index(r->index, &lock, COMMIT_LOCK) < 0)