]> git.ipfire.org Git - thirdparty/git.git/commitdiff
Merge branch 'jk/hash-object-fsck'
authorJunio C Hamano <gitster@pobox.com>
Mon, 30 Jan 2023 22:24:22 +0000 (14:24 -0800)
committerJunio C Hamano <gitster@pobox.com>
Mon, 30 Jan 2023 22:24:22 +0000 (14:24 -0800)
"git hash-object" now checks that the resulting object is well
formed with the same code as "git fsck".

* jk/hash-object-fsck:
  fsck: do not assume NUL-termination of buffers
  hash-object: use fsck for object checks
  fsck: provide a function to fsck buffer without object struct
  t: use hash-object --literally when created malformed objects
  t7030: stop using invalid tag name
  t1006: stop using 0-padded timestamps
  t1007: modernize malformed object tests

21 files changed:
fsck.c
fsck.h
object-file.c
t/t1006-cat-file.sh
t/t1007-hash-object.sh
t/t1450-fsck.sh
t/t1451-fsck-buffer.sh [new file with mode: 0755]
t/t4054-diff-bogus-tree.sh
t/t4058-diff-duplicates.sh
t/t4212-log-corrupt.sh
t/t5302-pack-index.sh
t/t5504-fetch-receive-strict.sh
t/t5702-protocol-v2.sh
t/t6300-for-each-ref.sh
t/t7030-verify-tag.sh
t/t7031-verify-tag-signed-ssh.sh
t/t7509-commit-authorship.sh
t/t7510-signed-commit.sh
t/t7528-signed-commit-ssh.sh
t/t8003-blame-corner-cases.sh
t/t9350-fast-export.sh

diff --git a/fsck.c b/fsck.c
index 47eaeedd7076ba60a621e072abb405127b3c33fe..2b18717ee805bc73aa50943731ac9a7212303d18 100644 (file)
--- a/fsck.c
+++ b/fsck.c
@@ -748,6 +748,23 @@ static int fsck_tree(const struct object_id *tree_oid,
        return retval;
 }
 
+/*
+ * Confirm that the headers of a commit or tag object end in a reasonable way,
+ * either with the usual "\n\n" separator, or at least with a trailing newline
+ * on the final header line.
+ *
+ * This property is important for the memory safety of our callers. It allows
+ * them to scan the buffer linewise without constantly checking the remaining
+ * size as long as:
+ *
+ *   - they check that there are bytes left in the buffer at the start of any
+ *     line (i.e., that the last newline they saw was not the final one we
+ *     found here)
+ *
+ *   - any intra-line scanning they do will stop at a newline, which will worst
+ *     case hit the newline we found here as the end-of-header. This makes it
+ *     OK for them to use helpers like parse_oid_hex(), or even skip_prefix().
+ */
 static int verify_headers(const void *data, unsigned long size,
                          const struct object_id *oid, enum object_type type,
                          struct fsck_options *options)
@@ -808,6 +825,20 @@ static int fsck_ident(const char **ident,
        if (*p != ' ')
                return report(options, oid, type, FSCK_MSG_MISSING_SPACE_BEFORE_DATE, "invalid author/committer line - missing space before date");
        p++;
+       /*
+        * Our timestamp parser is based on the C strto*() functions, which
+        * will happily eat whitespace, including the newline that is supposed
+        * to prevent us walking past the end of the buffer. So do our own
+        * scan, skipping linear whitespace but not newlines, and then
+        * confirming we found a digit. We _could_ be even more strict here,
+        * as we really expect only a single space, but since we have
+        * traditionally allowed extra whitespace, we'll continue to do so.
+        */
+       while (*p == ' ' || *p == '\t')
+               p++;
+       if (!isdigit(*p))
+               return report(options, oid, type, FSCK_MSG_BAD_DATE,
+                             "invalid author/committer line - bad date");
        if (*p == '0' && p[1] != ' ')
                return report(options, oid, type, FSCK_MSG_ZERO_PADDED_DATE, "invalid author/committer line - zero-padded date");
        if (date_overflows(parse_timestamp(p, &end, 10)))
@@ -834,12 +865,18 @@ static int fsck_commit(const struct object_id *oid,
        unsigned author_count;
        int err;
        const char *buffer_begin = buffer;
+       const char *buffer_end = buffer + size;
        const char *p;
 
+       /*
+        * We _must_ stop parsing immediately if this reports failure, as the
+        * memory safety of the rest of the function depends on it. See the
+        * comment above the definition of verify_headers() for more details.
+        */
        if (verify_headers(buffer, size, oid, OBJ_COMMIT, options))
                return -1;
 
-       if (!skip_prefix(buffer, "tree ", &buffer))
+       if (buffer >= buffer_end || !skip_prefix(buffer, "tree ", &buffer))
                return report(options, oid, OBJ_COMMIT, FSCK_MSG_MISSING_TREE, "invalid format - expected 'tree' line");
        if (parse_oid_hex(buffer, &tree_oid, &p) || *p != '\n') {
                err = report(options, oid, OBJ_COMMIT, FSCK_MSG_BAD_TREE_SHA1, "invalid 'tree' line format - bad sha1");
@@ -847,7 +884,7 @@ static int fsck_commit(const struct object_id *oid,
                        return err;
        }
        buffer = p + 1;
-       while (skip_prefix(buffer, "parent ", &buffer)) {
+       while (buffer < buffer_end && skip_prefix(buffer, "parent ", &buffer)) {
                if (parse_oid_hex(buffer, &parent_oid, &p) || *p != '\n') {
                        err = report(options, oid, OBJ_COMMIT, FSCK_MSG_BAD_PARENT_SHA1, "invalid 'parent' line format - bad sha1");
                        if (err)
@@ -856,7 +893,7 @@ static int fsck_commit(const struct object_id *oid,
                buffer = p + 1;
        }
        author_count = 0;
-       while (skip_prefix(buffer, "author ", &buffer)) {
+       while (buffer < buffer_end && skip_prefix(buffer, "author ", &buffer)) {
                author_count++;
                err = fsck_ident(&buffer, oid, OBJ_COMMIT, options);
                if (err)
@@ -868,7 +905,7 @@ static int fsck_commit(const struct object_id *oid,
                err = report(options, oid, OBJ_COMMIT, FSCK_MSG_MULTIPLE_AUTHORS, "invalid format - multiple 'author' lines");
        if (err)
                return err;
-       if (!skip_prefix(buffer, "committer ", &buffer))
+       if (buffer >= buffer_end || !skip_prefix(buffer, "committer ", &buffer))
                return report(options, oid, OBJ_COMMIT, FSCK_MSG_MISSING_COMMITTER, "invalid format - expected 'committer' line");
        err = fsck_ident(&buffer, oid, OBJ_COMMIT, options);
        if (err)
@@ -899,13 +936,19 @@ int fsck_tag_standalone(const struct object_id *oid, const char *buffer,
        int ret = 0;
        char *eol;
        struct strbuf sb = STRBUF_INIT;
+       const char *buffer_end = buffer + size;
        const char *p;
 
+       /*
+        * We _must_ stop parsing immediately if this reports failure, as the
+        * memory safety of the rest of the function depends on it. See the
+        * comment above the definition of verify_headers() for more details.
+        */
        ret = verify_headers(buffer, size, oid, OBJ_TAG, options);
        if (ret)
                goto done;
 
-       if (!skip_prefix(buffer, "object ", &buffer)) {
+       if (buffer >= buffer_end || !skip_prefix(buffer, "object ", &buffer)) {
                ret = report(options, oid, OBJ_TAG, FSCK_MSG_MISSING_OBJECT, "invalid format - expected 'object' line");
                goto done;
        }
@@ -916,11 +959,11 @@ int fsck_tag_standalone(const struct object_id *oid, const char *buffer,
        }
        buffer = p + 1;
 
-       if (!skip_prefix(buffer, "type ", &buffer)) {
+       if (buffer >= buffer_end || !skip_prefix(buffer, "type ", &buffer)) {
                ret = report(options, oid, OBJ_TAG, FSCK_MSG_MISSING_TYPE_ENTRY, "invalid format - expected 'type' line");
                goto done;
        }
-       eol = strchr(buffer, '\n');
+       eol = memchr(buffer, '\n', buffer_end - buffer);
        if (!eol) {
                ret = report(options, oid, OBJ_TAG, FSCK_MSG_MISSING_TYPE, "invalid format - unexpected end after 'type' line");
                goto done;
@@ -932,11 +975,11 @@ int fsck_tag_standalone(const struct object_id *oid, const char *buffer,
                goto done;
        buffer = eol + 1;
 
-       if (!skip_prefix(buffer, "tag ", &buffer)) {
+       if (buffer >= buffer_end || !skip_prefix(buffer, "tag ", &buffer)) {
                ret = report(options, oid, OBJ_TAG, FSCK_MSG_MISSING_TAG_ENTRY, "invalid format - expected 'tag' line");
                goto done;
        }
-       eol = strchr(buffer, '\n');
+       eol = memchr(buffer, '\n', buffer_end - buffer);
        if (!eol) {
                ret = report(options, oid, OBJ_TAG, FSCK_MSG_MISSING_TAG, "invalid format - unexpected end after 'type' line");
                goto done;
@@ -952,7 +995,7 @@ int fsck_tag_standalone(const struct object_id *oid, const char *buffer,
        }
        buffer = eol + 1;
 
-       if (!skip_prefix(buffer, "tagger ", &buffer)) {
+       if (buffer >= buffer_end || !skip_prefix(buffer, "tagger ", &buffer)) {
                /* early tags do not contain 'tagger' lines; warn only */
                ret = report(options, oid, OBJ_TAG, FSCK_MSG_MISSING_TAGGER_ENTRY, "invalid format - expected 'tagger' line");
                if (ret)
@@ -960,10 +1003,8 @@ int fsck_tag_standalone(const struct object_id *oid, const char *buffer,
        }
        else
                ret = fsck_ident(&buffer, oid, OBJ_TAG, options);
-       if (!*buffer)
-               goto done;
 
-       if (!starts_with(buffer, "\n")) {
+       if (buffer < buffer_end && !starts_with(buffer, "\n")) {
                /*
                 * The verify_headers() check will allow
                 * e.g. "[...]tagger <tagger>\nsome
@@ -1237,19 +1278,26 @@ int fsck_object(struct object *obj, void *data, unsigned long size,
        if (!obj)
                return report(options, NULL, OBJ_NONE, FSCK_MSG_BAD_OBJECT_SHA1, "no valid object to fsck");
 
-       if (obj->type == OBJ_BLOB)
-               return fsck_blob(&obj->oid, data, size, options);
-       if (obj->type == OBJ_TREE)
-               return fsck_tree(&obj->oid, data, size, options);
-       if (obj->type == OBJ_COMMIT)
-               return fsck_commit(&obj->oid, data, size, options);
-       if (obj->type == OBJ_TAG)
-               return fsck_tag(&obj->oid, data, size, options);
+       return fsck_buffer(&obj->oid, obj->type, data, size, options);
+}
+
+int fsck_buffer(const struct object_id *oid, enum object_type type,
+               void *data, unsigned long size,
+               struct fsck_options *options)
+{
+       if (type == OBJ_BLOB)
+               return fsck_blob(oid, data, size, options);
+       if (type == OBJ_TREE)
+               return fsck_tree(oid, data, size, options);
+       if (type == OBJ_COMMIT)
+               return fsck_commit(oid, data, size, options);
+       if (type == OBJ_TAG)
+               return fsck_tag(oid, data, size, options);
 
-       return report(options, &obj->oid, obj->type,
+       return report(options, oid, type,
                      FSCK_MSG_UNKNOWN_TYPE,
                      "unknown type '%d' (internal fsck error)",
-                     obj->type);
+                     type);
 }
 
 int fsck_error_function(struct fsck_options *o,
diff --git a/fsck.h b/fsck.h
index fcecf4101cfc9cbb60f9467d629403fe4221f782..668330880ef65f72ba4e9d38487152cc28c205a1 100644 (file)
--- a/fsck.h
+++ b/fsck.h
@@ -183,6 +183,14 @@ int fsck_walk(struct object *obj, void *data, struct fsck_options *options);
 int fsck_object(struct object *obj, void *data, unsigned long size,
        struct fsck_options *options);
 
+/*
+ * Same as fsck_object(), but for when the caller doesn't have an object
+ * struct.
+ */
+int fsck_buffer(const struct object_id *oid, enum object_type,
+               void *data, unsigned long size,
+               struct fsck_options *options);
+
 /*
  * fsck a tag, and pass info about it back to the caller. This is
  * exposed fsck_object() internals for git-mktag(1).
index ce9efae9940c78476f7610685ee4facaef8a0673..939865c1ae0566ba577861505f2ecf2e9fd19eeb 100644 (file)
@@ -33,6 +33,7 @@
 #include "object-store.h"
 #include "promisor-remote.h"
 #include "submodule.h"
+#include "fsck.h"
 
 /* The maximum size for an object header. */
 #define MAX_HEADER_LEN 32
@@ -2284,32 +2285,21 @@ int repo_has_object_file(struct repository *r,
        return repo_has_object_file_with_flags(r, oid, 0);
 }
 
-static void check_tree(const void *buf, size_t size)
-{
-       struct tree_desc desc;
-       struct name_entry entry;
-
-       init_tree_desc(&desc, buf, size);
-       while (tree_entry(&desc, &entry))
-               /* do nothing
-                * tree_entry() will die() on malformed entries */
-               ;
-}
-
-static void check_commit(const void *buf, size_t size)
-{
-       struct commit c;
-       memset(&c, 0, sizeof(c));
-       if (parse_commit_buffer(the_repository, &c, buf, size, 0))
-               die(_("corrupt commit"));
-}
-
-static void check_tag(const void *buf, size_t size)
-{
-       struct tag t;
-       memset(&t, 0, sizeof(t));
-       if (parse_tag_buffer(the_repository, &t, buf, size))
-               die(_("corrupt tag"));
+/*
+ * We can't use the normal fsck_error_function() for index_mem(),
+ * because we don't yet have a valid oid for it to report. Instead,
+ * report the minimal fsck error here, and rely on the caller to
+ * give more context.
+ */
+static int hash_format_check_report(struct fsck_options *opts,
+                                    const struct object_id *oid,
+                                    enum object_type object_type,
+                                    enum fsck_msg_type msg_type,
+                                    enum fsck_msg_id msg_id,
+                                    const char *message)
+{
+       error(_("object fails fsck: %s"), message);
+       return 1;
 }
 
 static int index_mem(struct index_state *istate,
@@ -2336,12 +2326,13 @@ static int index_mem(struct index_state *istate,
                }
        }
        if (flags & HASH_FORMAT_CHECK) {
-               if (type == OBJ_TREE)
-                       check_tree(buf, size);
-               if (type == OBJ_COMMIT)
-                       check_commit(buf, size);
-               if (type == OBJ_TAG)
-                       check_tag(buf, size);
+               struct fsck_options opts = FSCK_OPTIONS_DEFAULT;
+
+               opts.strict = 1;
+               opts.error_func = hash_format_check_report;
+               if (fsck_buffer(null_oid(), type, buf, size, &opts))
+                       die(_("refusing to create malformed object"));
+               fsck_finish(&opts);
        }
 
        if (write_object)
index 23b8942edba45c32822886ee93acef9b5474ea02..2d875b17d8a94a45b9789e3466139adbd6b4c680 100755 (executable)
@@ -292,8 +292,8 @@ commit_message="Initial commit"
 commit_sha1=$(echo_without_newline "$commit_message" | git commit-tree $tree_sha1)
 commit_size=$(($(test_oid hexsz) + 137))
 commit_content="tree $tree_sha1
-author $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> 0000000000 +0000
-committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 0000000000 +0000
+author $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> 0 +0000
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 0 +0000
 
 $commit_message"
 
@@ -304,7 +304,7 @@ type blob
 tag hellotag
 tagger $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
 tag_description="This is a tag"
-tag_content="$tag_header_without_timestamp 0000000000 +0000
+tag_content="$tag_header_without_timestamp 0 +0000
 
 $tag_description"
 
index ac5ad8c7402d2bd3f43d38e5a676d864ea415d37..ac3d173767ae72be7bc43e4df269d88fb040f7ba 100755 (executable)
@@ -203,23 +203,34 @@ done
 test_expect_success 'too-short tree' '
        echo abc >malformed-tree &&
        test_must_fail git hash-object -t tree malformed-tree 2>err &&
-       test_i18ngrep "too-short tree object" err
+       grep "too-short tree object" err
 '
 
 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 &&
+       hex_oid=$(echo foo | git hash-object --stdin -w) &&
+       bin_oid=$(echo $hex_oid | hex2oct) &&
+       printf "9100644 \0$bin_oid" >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
+       grep "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 &&
+       hex_oid=$(echo foo | git hash-object --stdin -w) &&
+       bin_oid=$(echo $hex_oid | hex2oct) &&
+       printf "100644 \0$bin_oid" >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
+       grep "empty filename in tree entry" err
+'
+
+test_expect_success 'duplicate filename in tree' '
+       hex_oid=$(echo foo | git hash-object --stdin -w) &&
+       bin_oid=$(echo $hex_oid | hex2oct) &&
+       {
+               printf "100644 file\0$bin_oid" &&
+               printf "100644 file\0$bin_oid"
+       } >tree-with-duplicate-filename &&
+       test_must_fail git hash-object -t tree tree-with-duplicate-filename 2>err &&
+       grep "duplicateEntries" err
 '
 
 test_expect_success 'corrupt commit' '
index de0f6d5e7fd49ca5276696833d8f9dd034a15629..fdb886dfe431f36dfc4ef6bcf24abf6827c2243d 100755 (executable)
@@ -212,7 +212,7 @@ test_expect_success 'email without @ is okay' '
 test_expect_success 'email with embedded > is not okay' '
        git cat-file commit HEAD >basis &&
        sed "s/@[a-z]/&>/" basis >bad-email &&
-       new=$(git hash-object -t commit -w --stdin <bad-email) &&
+       new=$(git hash-object --literally -t commit -w --stdin <bad-email) &&
        test_when_finished "remove_object $new" &&
        git update-ref refs/heads/bogus "$new" &&
        test_when_finished "git update-ref -d refs/heads/bogus" &&
@@ -223,7 +223,7 @@ test_expect_success 'email with embedded > is not okay' '
 test_expect_success 'missing < email delimiter is reported nicely' '
        git cat-file commit HEAD >basis &&
        sed "s/<//" basis >bad-email-2 &&
-       new=$(git hash-object -t commit -w --stdin <bad-email-2) &&
+       new=$(git hash-object --literally -t commit -w --stdin <bad-email-2) &&
        test_when_finished "remove_object $new" &&
        git update-ref refs/heads/bogus "$new" &&
        test_when_finished "git update-ref -d refs/heads/bogus" &&
@@ -234,7 +234,7 @@ test_expect_success 'missing < email delimiter is reported nicely' '
 test_expect_success 'missing email is reported nicely' '
        git cat-file commit HEAD >basis &&
        sed "s/[a-z]* <[^>]*>//" basis >bad-email-3 &&
-       new=$(git hash-object -t commit -w --stdin <bad-email-3) &&
+       new=$(git hash-object --literally -t commit -w --stdin <bad-email-3) &&
        test_when_finished "remove_object $new" &&
        git update-ref refs/heads/bogus "$new" &&
        test_when_finished "git update-ref -d refs/heads/bogus" &&
@@ -245,7 +245,7 @@ test_expect_success 'missing email is reported nicely' '
 test_expect_success '> in name is reported' '
        git cat-file commit HEAD >basis &&
        sed "s/ </> </" basis >bad-email-4 &&
-       new=$(git hash-object -t commit -w --stdin <bad-email-4) &&
+       new=$(git hash-object --literally -t commit -w --stdin <bad-email-4) &&
        test_when_finished "remove_object $new" &&
        git update-ref refs/heads/bogus "$new" &&
        test_when_finished "git update-ref -d refs/heads/bogus" &&
@@ -258,7 +258,7 @@ test_expect_success 'integer overflow in timestamps is reported' '
        git cat-file commit HEAD >basis &&
        sed "s/^\\(author .*>\\) [0-9]*/\\1 18446744073709551617/" \
                <basis >bad-timestamp &&
-       new=$(git hash-object -t commit -w --stdin <bad-timestamp) &&
+       new=$(git hash-object --literally -t commit -w --stdin <bad-timestamp) &&
        test_when_finished "remove_object $new" &&
        git update-ref refs/heads/bogus "$new" &&
        test_when_finished "git update-ref -d refs/heads/bogus" &&
@@ -269,7 +269,7 @@ test_expect_success 'integer overflow in timestamps is reported' '
 test_expect_success 'commit with NUL in header' '
        git cat-file commit HEAD >basis &&
        sed "s/author ./author Q/" <basis | q_to_nul >commit-NUL-header &&
-       new=$(git hash-object -t commit -w --stdin <commit-NUL-header) &&
+       new=$(git hash-object --literally -t commit -w --stdin <commit-NUL-header) &&
        test_when_finished "remove_object $new" &&
        git update-ref refs/heads/bogus "$new" &&
        test_when_finished "git update-ref -d refs/heads/bogus" &&
@@ -292,7 +292,7 @@ test_expect_success 'tree object with duplicate entries' '
                        git cat-file tree $T &&
                        git cat-file tree $T
                ) |
-               git hash-object -w -t tree --stdin
+               git hash-object --literally -w -t tree --stdin
        ) &&
        test_must_fail git fsck 2>out &&
        test_i18ngrep "error in tree .*contains duplicate file entries" out
@@ -426,7 +426,7 @@ test_expect_success 'tag with incorrect tag name & missing tagger' '
        This is an invalid tag.
        EOF
 
-       tag=$(git hash-object -t tag -w --stdin <wrong-tag) &&
+       tag=$(git hash-object --literally -t tag -w --stdin <wrong-tag) &&
        test_when_finished "remove_object $tag" &&
        echo $tag >.git/refs/tags/wrong &&
        test_when_finished "git update-ref -d refs/tags/wrong" &&
@@ -558,7 +558,7 @@ test_expect_success 'rev-list --verify-objects with commit graph (parent)' '
 test_expect_success 'force fsck to ignore double author' '
        git cat-file commit HEAD >basis &&
        sed "s/^author .*/&,&/" <basis | tr , \\n >multiple-authors &&
-       new=$(git hash-object -t commit -w --stdin <multiple-authors) &&
+       new=$(git hash-object --literally -t commit -w --stdin <multiple-authors) &&
        test_when_finished "remove_object $new" &&
        git update-ref refs/heads/bogus "$new" &&
        test_when_finished "git update-ref -d refs/heads/bogus" &&
@@ -573,7 +573,7 @@ test_expect_success 'fsck notices blob entry pointing to null sha1' '
        (git init null-blob &&
         cd null-blob &&
         sha=$(printf "100644 file$_bz$_bzoid" |
-              git hash-object -w --stdin -t tree) &&
+              git hash-object --literally -w --stdin -t tree) &&
          git fsck 2>out &&
          test_i18ngrep "warning.*null sha1" out
        )
@@ -583,7 +583,7 @@ test_expect_success 'fsck notices submodule entry pointing to null sha1' '
        (git init null-commit &&
         cd null-commit &&
         sha=$(printf "160000 submodule$_bz$_bzoid" |
-              git hash-object -w --stdin -t tree) &&
+              git hash-object --literally -w --stdin -t tree) &&
          git fsck 2>out &&
          test_i18ngrep "warning.*null sha1" out
        )
@@ -648,7 +648,7 @@ test_expect_success 'NUL in commit' '
                git commit --allow-empty -m "initial commitQNUL after message" &&
                git cat-file commit HEAD >original &&
                q_to_nul <original >munged &&
-               git hash-object -w -t commit --stdin <munged >name &&
+               git hash-object --literally -w -t commit --stdin <munged >name &&
                git branch bad $(cat name) &&
 
                test_must_fail git -c fsck.nulInCommit=error fsck 2>warn.1 &&
@@ -794,8 +794,8 @@ test_expect_success 'fsck errors in packed objects' '
        git cat-file commit HEAD >basis &&
        sed "s/</one/" basis >one &&
        sed "s/</foo/" basis >two &&
-       one=$(git hash-object -t commit -w one) &&
-       two=$(git hash-object -t commit -w two) &&
+       one=$(git hash-object --literally -t commit -w one) &&
+       two=$(git hash-object --literally -t commit -w two) &&
        pack=$(
                {
                        echo $one &&
diff --git a/t/t1451-fsck-buffer.sh b/t/t1451-fsck-buffer.sh
new file mode 100755 (executable)
index 0000000..9ac270a
--- /dev/null
@@ -0,0 +1,140 @@
+#!/bin/sh
+
+test_description='fsck on buffers without NUL termination
+
+The goal here is to make sure that the various fsck parsers never look
+past the end of the buffer they are given, even when encountering broken
+or truncated objects.
+
+We have to use "hash-object" for this because most code paths that read objects
+append an extra NUL for safety after the buffer. But hash-object, since it is
+reading straight from a file (and possibly even mmap-ing it) cannot always do
+so.
+
+These tests _might_ catch such overruns in normal use, but should be run with
+ASan or valgrind for more confidence.
+'
+. ./test-lib.sh
+
+# the general idea for tags and commits is to build up the "base" file
+# progressively, and then test new truncations on top of it.
+reset () {
+       test_expect_success 'reset input to empty' '
+               >base
+       '
+}
+
+add () {
+       content="$1"
+       type=${content%% *}
+       test_expect_success "add $type line" '
+               echo "$content" >>base
+       '
+}
+
+check () {
+       type=$1
+       fsck=$2
+       content=$3
+       test_expect_success "truncated $type ($fsck, \"$content\")" '
+               # do not pipe into hash-object here; we want to increase
+               # the chance that it uses a fixed-size buffer or mmap,
+               # and a pipe would be read into a strbuf.
+               {
+                       cat base &&
+                       echo "$content"
+               } >input &&
+               test_must_fail git hash-object -t "$type" input 2>err &&
+               grep "$fsck" err
+       '
+}
+
+test_expect_success 'create valid objects' '
+       git commit --allow-empty -m foo &&
+       commit=$(git rev-parse --verify HEAD) &&
+       tree=$(git rev-parse --verify HEAD^{tree})
+'
+
+reset
+check commit missingTree ""
+check commit missingTree "tr"
+check commit missingTree "tree"
+check commit badTreeSha1 "tree "
+check commit badTreeSha1 "tree 1234"
+add "tree $tree"
+
+# these expect missingAuthor because "parent" is optional
+check commit missingAuthor ""
+check commit missingAuthor "par"
+check commit missingAuthor "parent"
+check commit badParentSha1 "parent "
+check commit badParentSha1 "parent 1234"
+add "parent $commit"
+
+check commit missingAuthor ""
+check commit missingAuthor "au"
+check commit missingAuthor "author"
+ident_checks () {
+       check $1 missingEmail "$2 "
+       check $1 missingEmail "$2 name"
+       check $1 badEmail "$2 name <"
+       check $1 badEmail "$2 name <email"
+       check $1 missingSpaceBeforeDate "$2 name <email>"
+       check $1 badDate "$2 name <email> "
+       check $1 badDate "$2 name <email> 1234"
+       check $1 badTimezone "$2 name <email> 1234 "
+       check $1 badTimezone "$2 name <email> 1234 +"
+}
+ident_checks commit author
+add "author name <email> 1234 +0000"
+
+check commit missingCommitter ""
+check commit missingCommitter "co"
+check commit missingCommitter "committer"
+ident_checks commit committer
+add "committer name <email> 1234 +0000"
+
+reset
+check tag missingObject ""
+check tag missingObject "obj"
+check tag missingObject "object"
+check tag badObjectSha1 "object "
+check tag badObjectSha1 "object 1234"
+add "object $commit"
+
+check tag missingType ""
+check tag missingType "ty"
+check tag missingType "type"
+check tag badType "type "
+check tag badType "type com"
+add "type commit"
+
+check tag missingTagEntry ""
+check tag missingTagEntry "ta"
+check tag missingTagEntry "tag"
+check tag badTagName "tag "
+add "tag foo"
+
+check tag missingTagger ""
+check tag missingTagger "ta"
+check tag missingTagger "tagger"
+ident_checks tag tagger
+
+# trees are a binary format and can't use our earlier helpers
+test_expect_success 'truncated tree (short hash)' '
+       printf "100644 foo\0\1\1\1\1" >input &&
+       test_must_fail git hash-object -t tree input 2>err &&
+       grep badTree err
+'
+
+test_expect_success 'truncated tree (missing nul)' '
+       # these two things are indistinguishable to the parser. The important
+       # thing about this is example is that there are enough bytes to
+       # make up a hash, and that there is no NUL (and we confirm that the
+       # parser does not walk past the end of the buffer).
+       printf "100644 a long filename, or a hash with missing nul?" >input &&
+       test_must_fail git hash-object -t tree input 2>err &&
+       grep badTree err
+'
+
+test_done
index 294fb5531372d597a5a551b40e66437d7599e766..05c88f8cdf01d98b61eacff08b32cb57320e7d3b 100755 (executable)
@@ -10,7 +10,7 @@ test_expect_success 'create bogus tree' '
        bogus_tree=$(
                printf "100644 fooQ$name" |
                q_to_nul |
-               git hash-object -w --stdin -t tree
+               git hash-object --literally -w --stdin -t tree
        )
 '
 
index 54614b814dbc858e15fd82b8bc563ab9aae5fae8..2501c89c1c91ec7ddc02f4e285aa4c10c32b98f6 100755 (executable)
@@ -29,7 +29,7 @@ make_tree () {
                make_tree_entry "$1" "$2" "$3"
                shift; shift; shift
        done |
-       git hash-object -w -t tree --stdin
+       git hash-object --literally -w -t tree --stdin
 }
 
 # this is kind of a convoluted setup, but matches
index 30a219894bb45f2655596fe02e8a7d001a35eabd..e89e1f54b6caed2e97d04aaf4672a0491a398acf 100755 (executable)
@@ -10,7 +10,7 @@ test_expect_success 'setup' '
 
        git cat-file commit HEAD |
        sed "/^author /s/>/>-<>/" >broken_email.commit &&
-       git hash-object -w -t commit broken_email.commit >broken_email.hash &&
+       git hash-object --literally -w -t commit broken_email.commit >broken_email.hash &&
        git update-ref refs/heads/broken_email $(cat broken_email.hash)
 '
 
@@ -46,7 +46,7 @@ test_expect_success 'git log --format with broken author email' '
 munge_author_date () {
        git cat-file commit "$1" >commit.orig &&
        sed "s/^\(author .*>\) [0-9]*/\1 $2/" <commit.orig >commit.munge &&
-       git hash-object -w -t commit commit.munge
+       git hash-object --literally -w -t commit commit.munge
 }
 
 test_expect_success 'unparsable dates produce sentinel value' '
index b0095ab41d30aadda45c7dd6e812fd0a5f40ab26..59e9e77223b3e25d07d2dd7052e7674221c8a78d 100755 (executable)
@@ -263,7 +263,7 @@ tag guten tag
 This is an invalid tag.
 EOF
 
-       tag=$(git hash-object -t tag -w --stdin <wrong-tag) &&
+       tag=$(git hash-object -t tag -w --stdin --literally <wrong-tag) &&
        pack1=$(echo $tag $sha | git pack-objects tag-test) &&
        echo remove tag object &&
        thirtyeight=${tag#??} &&
index ac4099ca8931930989eece26fa735e2f6c00bbc3..88d3c56750accc7376614f0fda48d85ddf298cc6 100755 (executable)
@@ -138,7 +138,7 @@ This commit object intentionally broken
 EOF
 
 test_expect_success 'setup bogus commit' '
-       commit="$(git hash-object -t commit -w --stdin <bogus-commit)"
+       commit="$(git hash-object --literally -t commit -w --stdin <bogus-commit)"
 '
 
 test_expect_success 'fsck with no skipList input' '
index b33cd4afca3d021f1f8f38393521589243ae95fb..e4db7513f42192895e33e7eec431c73bec0007d8 100755 (executable)
@@ -1114,7 +1114,7 @@ test_expect_success 'packfile-uri with transfer.fsckobjects fails on bad object'
 
        This commit object intentionally broken
        EOF
-       BOGUS=$(git -C "$P" hash-object -t commit -w --stdin <bogus-commit) &&
+       BOGUS=$(git -C "$P" hash-object -t commit -w --stdin --literally <bogus-commit) &&
        git -C "$P" branch bogus-branch "$BOGUS" &&
 
        echo my-blob >"$P/my-blob" &&
index 2ae1fc721b1c477e8830d15eeda323c7e4b9bb7a..c466fd989f168b0fc696e83516474f11040a58be 100755 (executable)
@@ -606,7 +606,7 @@ test_expect_success 'create tag without tagger' '
        git tag -a -m "Broken tag" taggerless &&
        git tag -f taggerless $(git cat-file tag taggerless |
                sed -e "/^tagger /d" |
-               git hash-object --stdin -w -t tag)
+               git hash-object --literally --stdin -w -t tag)
 '
 
 test_atom refs/tags/taggerless type 'commit'
index 10faa645157ea4521e370abdfd470c84a24f6051..6f526c37c2776e6288a1abe5860b6a9efed25183 100755 (executable)
@@ -115,7 +115,7 @@ test_expect_success GPGSM 'verify and show signatures x509 with high minTrustLev
 
 test_expect_success GPG 'detect fudged signature' '
        git cat-file tag seventh-signed >raw &&
-       sed -e "/^tag / s/seventh/7th forged/" raw >forged1 &&
+       sed -e "/^tag / s/seventh/7th-forged/" raw >forged1 &&
        git hash-object -w -t tag forged1 >forged1.tag &&
        test_must_fail git verify-tag $(cat forged1.tag) 2>actual1 &&
        grep "BAD signature from" actual1 &&
index 1cb36b9ab83cdba01a554a94c982c968cd572f8b..36eb86a4b19f3c758b2830fe5157a1667069321a 100755 (executable)
@@ -125,7 +125,7 @@ test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'verify-tag failes with tag date ou
 test_expect_success GPGSSH 'detect fudged ssh signature' '
        test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
        git cat-file tag seventh-signed >raw &&
-       sed -e "/^tag / s/seventh/7th forged/" raw >forged1 &&
+       sed -e "/^tag / s/seventh/7th-forged/" raw >forged1 &&
        git hash-object -w -t tag forged1 >forged1.tag &&
        test_must_fail git verify-tag $(cat forged1.tag) 2>actual1 &&
        grep "${GPGSSH_BAD_SIGNATURE}" actual1 &&
index 21c668f75ed7345430f88f65b7e9072a873b9c42..5d890949f75b5acb4ef9334a41c080ece60c1653 100755 (executable)
@@ -105,7 +105,7 @@ test_expect_success '--amend option with empty author' '
 test_expect_success '--amend option with missing author' '
        git cat-file commit Initial >tmp &&
        sed "s/author [^<]* </author </" tmp >malformed &&
-       sha=$(git hash-object -t commit -w malformed) &&
+       sha=$(git hash-object --literally -t commit -w malformed) &&
        test_when_finished "remove_object $sha" &&
        git checkout $sha &&
        test_when_finished "git checkout Initial" &&
index 8593b7e3cb8d9aa0c4badeef4273ed406cecc9ca..bc7a31ba3e7fc4c0611c23f60470fcb392cdd424 100755 (executable)
@@ -202,7 +202,7 @@ test_expect_success GPG 'detect fudged signature with NUL' '
        git cat-file commit seventh-signed >raw &&
        cat raw >forged2 &&
        echo Qwik | tr "Q" "\000" >>forged2 &&
-       git hash-object -w -t commit forged2 >forged2.commit &&
+       git hash-object --literally -w -t commit forged2 >forged2.commit &&
        test_must_fail git verify-commit $(cat forged2.commit) &&
        git show --pretty=short --show-signature $(cat forged2.commit) >actual2 &&
        grep "BAD signature from" actual2 &&
index f47e99517983163e0bbb3fce9c35800270fe4e28..065f78063629cbfad15e9360836544da34a16484 100755 (executable)
@@ -270,7 +270,7 @@ test_expect_success GPGSSH 'detect fudged signature with NUL' '
        git cat-file commit seventh-signed >raw &&
        cat raw >forged2 &&
        echo Qwik | tr "Q" "\000" >>forged2 &&
-       git hash-object -w -t commit forged2 >forged2.commit &&
+       git hash-object --literally -w -t commit forged2 >forged2.commit &&
        test_must_fail git verify-commit $(cat forged2.commit) &&
        git show --pretty=short --show-signature $(cat forged2.commit) >actual2 &&
        grep "${GPGSSH_BAD_SIGNATURE}" actual2 &&
index d751d48b7dae6a3e007d5eb19f92cd41eb445065..8bcd39e81bfb7f28768c988caf16f27773460b4e 100755 (executable)
@@ -201,7 +201,7 @@ committer David Reiss <dreiss@facebook.com> 1234567890 +0000
 
 some message
 EOF
-  COMMIT=$(git hash-object -t commit -w badcommit) &&
+  COMMIT=$(git hash-object --literally -t commit -w badcommit) &&
   git --no-pager blame $COMMIT -- uno >/dev/null
 '
 
index ff21a12ee6e4eb1557eca22bb60ab2e01aa6b909..26c25c0eb2ba57fa90c682950fda4899329474f8 100755 (executable)
@@ -373,7 +373,7 @@ EOF
 
 test_expect_success 'cope with tagger-less tags' '
 
-       TAG=$(git hash-object -t tag -w tag-content) &&
+       TAG=$(git hash-object --literally -t tag -w tag-content) &&
        git update-ref refs/tags/sonnenschein $TAG &&
        git fast-export -C -C --signed-tags=strip --all > output &&
        test $(grep -c "^tag " output) = 4 &&