diff options
| -rw-r--r-- | fsck.c | 18 | ||||
| -rwxr-xr-x | t/t1007-hash-object.sh | 25 | ||||
| -rwxr-xr-x | t/t1450-fsck.sh | 16 | ||||
| -rw-r--r-- | tree-walk.c | 83 | ||||
| -rw-r--r-- | tree-walk.h | 8 |
5 files changed, 130 insertions, 20 deletions
@@ -347,8 +347,9 @@ static int fsck_walk_tree(struct tree *tree, void *data, struct fsck_options *op return -1; name = get_object_name(options, &tree->object); - init_tree_desc(&desc, tree->buffer, tree->size); - while (tree_entry(&desc, &entry)) { + if (init_tree_desc_gently(&desc, tree->buffer, tree->size)) + return -1; + while (tree_entry_gently(&desc, &entry)) { struct object *obj; int result; @@ -520,7 +521,7 @@ static int verify_ordered(unsigned mode1, const char *name1, unsigned mode2, con static int fsck_tree(struct tree *item, struct fsck_options *options) { - int retval; + int retval = 0; int has_null_sha1 = 0; int has_full_path = 0; int has_empty_name = 0; @@ -535,7 +536,10 @@ static int fsck_tree(struct tree *item, struct fsck_options *options) unsigned o_mode; const char *o_name; - init_tree_desc(&desc, item->buffer, item->size); + if (init_tree_desc_gently(&desc, item->buffer, item->size)) { + retval += report(options, &item->object, FSCK_MSG_BAD_TREE, "cannot be parsed as a tree"); + return retval; + } o_mode = 0; o_name = NULL; @@ -556,7 +560,10 @@ static int fsck_tree(struct tree *item, struct fsck_options *options) is_hfs_dotgit(name) || is_ntfs_dotgit(name)); has_zero_pad |= *(char *)desc.buffer == '0'; - update_tree_entry(&desc); + if (update_tree_entry_gently(&desc)) { + retval += report(options, &item->object, FSCK_MSG_BAD_TREE, "cannot be parsed as a tree"); + break; + } switch (mode) { /* @@ -597,7 +604,6 @@ static int fsck_tree(struct tree *item, struct fsck_options *options) o_name = name; } - retval = 0; if (has_null_sha1) retval += report(options, &item->object, FSCK_MSG_NULL_SHA1, "contains entries pointing to null sha1"); if (has_full_path) diff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh index acca9ac562..c5245c5cb4 100755 --- a/t/t1007-hash-object.sh +++ b/t/t1007-hash-object.sh @@ -183,9 +183,30 @@ for args in "-w --stdin-paths" "--stdin-paths -w"; do pop_repo done -test_expect_success 'corrupt tree' ' +test_expect_success 'too-short tree' ' echo abc >malformed-tree && - test_must_fail git hash-object -t tree malformed-tree + test_must_fail git hash-object -t tree malformed-tree 2>err && + test_i18ngrep "too-short tree object" err +' + +hex2oct() { + perl -ne 'printf "\\%03o", hex for /../g' +} + +test_expect_success 'malformed mode in tree' ' + hex_sha1=$(echo foo | git hash-object --stdin -w) && + bin_sha1=$(echo $hex_sha1 | hex2oct) && + printf "9100644 \0$bin_sha1" >tree-with-malformed-mode && + test_must_fail git hash-object -t tree tree-with-malformed-mode 2>err && + test_i18ngrep "malformed mode in tree entry" err +' + +test_expect_success 'empty filename in tree' ' + hex_sha1=$(echo foo | git hash-object --stdin -w) && + bin_sha1=$(echo $hex_sha1 | hex2oct) && + printf "100644 \0$bin_sha1" >tree-with-empty-filename && + test_must_fail git hash-object -t tree tree-with-empty-filename 2>err && + test_i18ngrep "empty filename in tree entry" err ' test_expect_success 'corrupt commit' ' diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh index 8f52da2771..ee7d4736db 100755 --- a/t/t1450-fsck.sh +++ b/t/t1450-fsck.sh @@ -188,8 +188,7 @@ test_expect_success 'commit with NUL in header' ' grep "error in commit $new.*unterminated header: NUL at offset" out ' -test_expect_success 'malformatted tree object' ' - test_when_finished "git update-ref -d refs/tags/wrong" && +test_expect_success 'tree object with duplicate entries' ' test_when_finished "remove_object \$T" && T=$( GIT_INDEX_FILE=test-index && @@ -208,6 +207,19 @@ test_expect_success 'malformatted tree object' ' grep "error in tree .*contains duplicate file entries" out ' +test_expect_success 'unparseable tree object' ' + test_when_finished "git update-ref -d refs/heads/wrong" && + test_when_finished "remove_object \$tree_sha1" && + test_when_finished "remove_object \$commit_sha1" && + tree_sha1=$(printf "100644 \0twenty-bytes-of-junk" | git hash-object -t tree --stdin -w --literally) && + commit_sha1=$(git commit-tree $tree_sha1) && + git update-ref refs/heads/wrong $commit_sha1 && + test_must_fail git fsck 2>out && + test_i18ngrep "error: empty filename in tree entry" out && + test_i18ngrep "$tree_sha1" out && + test_i18ngrep ! "fatal: empty filename in tree entry" out +' + test_expect_success 'tag pointing to nonexistent' ' cat >invalid-tag <<-\EOF && object ffffffffffffffffffffffffffffffffffffffff diff --git a/tree-walk.c b/tree-walk.c index ce27842439..828f4356be 100644 --- a/tree-walk.c +++ b/tree-walk.c @@ -22,31 +22,60 @@ static const char *get_mode(const char *str, unsigned int *modep) return str; } -static void decode_tree_entry(struct tree_desc *desc, const char *buf, unsigned long size) +static int decode_tree_entry(struct tree_desc *desc, const char *buf, unsigned long size, struct strbuf *err) { const char *path; unsigned int mode, len; - if (size < 24 || buf[size - 21]) - die("corrupt tree file"); + if (size < 23 || buf[size - 21]) { + strbuf_addstr(err, _("too-short tree object")); + return -1; + } path = get_mode(buf, &mode); - if (!path || !*path) - die("corrupt tree file"); + if (!path) { + strbuf_addstr(err, _("malformed mode in tree entry")); + return -1; + } + if (!*path) { + strbuf_addstr(err, _("empty filename in tree entry")); + return -1; + } len = strlen(path) + 1; /* Initialize the descriptor entry */ desc->entry.path = path; desc->entry.mode = canon_mode(mode); desc->entry.oid = (const struct object_id *)(path + len); + + return 0; } -void init_tree_desc(struct tree_desc *desc, const void *buffer, unsigned long size) +static int init_tree_desc_internal(struct tree_desc *desc, const void *buffer, unsigned long size, struct strbuf *err) { desc->buffer = buffer; desc->size = size; if (size) - decode_tree_entry(desc, buffer, size); + return decode_tree_entry(desc, buffer, size, err); + return 0; +} + +void init_tree_desc(struct tree_desc *desc, const void *buffer, unsigned long size) +{ + struct strbuf err = STRBUF_INIT; + if (init_tree_desc_internal(desc, buffer, size, &err)) + die("%s", err.buf); + strbuf_release(&err); +} + +int init_tree_desc_gently(struct tree_desc *desc, const void *buffer, unsigned long size) +{ + struct strbuf err = STRBUF_INIT; + int result = init_tree_desc_internal(desc, buffer, size, &err); + if (result) + error("%s", err.buf); + strbuf_release(&err); + return result; } void *fill_tree_descriptor(struct tree_desc *desc, const unsigned char *sha1) @@ -73,7 +102,7 @@ static void entry_extract(struct tree_desc *t, struct name_entry *a) *a = t->entry; } -void update_tree_entry(struct tree_desc *desc) +static int update_tree_entry_internal(struct tree_desc *desc, struct strbuf *err) { const void *buf = desc->buffer; const unsigned char *end = desc->entry.oid->hash + 20; @@ -81,13 +110,36 @@ void update_tree_entry(struct tree_desc *desc) unsigned long len = end - (const unsigned char *)buf; if (size < len) - die("corrupt tree file"); + die(_("too-short tree file")); buf = end; size -= len; desc->buffer = buf; desc->size = size; if (size) - decode_tree_entry(desc, buf, size); + return decode_tree_entry(desc, buf, size, err); + return 0; +} + +void update_tree_entry(struct tree_desc *desc) +{ + struct strbuf err = STRBUF_INIT; + if (update_tree_entry_internal(desc, &err)) + die("%s", err.buf); + strbuf_release(&err); +} + +int update_tree_entry_gently(struct tree_desc *desc) +{ + struct strbuf err = STRBUF_INIT; + if (update_tree_entry_internal(desc, &err)) { + error("%s", err.buf); + strbuf_release(&err); + /* Stop processing this tree after error */ + desc->size = 0; + return -1; + } + strbuf_release(&err); + return 0; } int tree_entry(struct tree_desc *desc, struct name_entry *entry) @@ -100,6 +152,17 @@ int tree_entry(struct tree_desc *desc, struct name_entry *entry) return 1; } +int tree_entry_gently(struct tree_desc *desc, struct name_entry *entry) +{ + if (!desc->size) + return 0; + + *entry = desc->entry; + if (update_tree_entry_gently(desc)) + return 0; + return 1; +} + void setup_traverse_info(struct traverse_info *info, const char *base) { int pathlen = strlen(base); diff --git a/tree-walk.h b/tree-walk.h index 97a7d6957e..68bb78b928 100644 --- a/tree-walk.h +++ b/tree-walk.h @@ -25,14 +25,22 @@ static inline int tree_entry_len(const struct name_entry *ne) return (const char *)ne->oid - ne->path - 1; } +/* + * The _gently versions of these functions warn and return false on a + * corrupt tree entry rather than dying, + */ + void update_tree_entry(struct tree_desc *); +int update_tree_entry_gently(struct tree_desc *); void init_tree_desc(struct tree_desc *desc, const void *buf, unsigned long size); +int init_tree_desc_gently(struct tree_desc *desc, const void *buf, unsigned long size); /* * Helper function that does both tree_entry_extract() and update_tree_entry() * and returns true for success */ int tree_entry(struct tree_desc *, struct name_entry *); +int tree_entry_gently(struct tree_desc *, struct name_entry *); void *fill_tree_descriptor(struct tree_desc *desc, const unsigned char *sha1); |