git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: git@vger.kernel.org
Cc: avarab@gmail.com, gitster@pobox.com, peff@peff.net,
	sunshine@sunshineco.com, szeder.dev@gmail.com
Subject: [PATCH v2 0/7] harden unexpected object types checks
Date: Tue, 9 Apr 2019 19:13:06 -0700	[thread overview]
Message-ID: <cover.1554861974.git.me@ttaylorr.com> (raw)
In-Reply-To: <cover.1554435033.git.me@ttaylorr.com>

Hi everyone,

Here is 'v2' of mine and Peff's series to add tests for and fix some
cases when object connections are malformed in various ways.

There aren't many major changes from the initial version, but what has
changed is described briefly here, and in complete detail in the
range-diff at the bottom of this message:

  - [2/7] was amended with Peff's suggestion to be a little clearer about
    what 'lone' and 'seen' mean

  - [2/7] was changed to replace all instances of:

      git cat-file ... | sed -e ... >broken-obj

    with:

      git cat-file ... >good-obj &&
      sed -e ... <good-obj >broken-obj

  - [4/7] was cleaned up to include the right test after a faulty
    rebase.

Thanks as always for your review :-).


Thanks,
Taylor

Jeff King (3):
  get_commit_tree(): return NULL for broken tree
  rev-list: let traversal die when --missing is not in use
  rev-list: detect broken root trees

Taylor Blau (4):
  t: move 'hex2oct' into test-lib-functions.sh
  t: introduce tests for unexpected object types
  list-objects.c: handle unexpected non-blob entries
  list-objects.c: handle unexpected non-tree entries

 builtin/rev-list.c                     |   4 +-
 commit.c                               |   6 +-
 list-objects.c                         |  13 +++
 t/t1007-hash-object.sh                 |   4 -
 t/t1450-fsck.sh                        |   4 -
 t/t5601-clone.sh                       |   4 -
 t/t6102-rev-list-unexpected-objects.sh | 127 +++++++++++++++++++++++++
 t/test-lib-functions.sh                |   6 ++
 8 files changed, 152 insertions(+), 16 deletions(-)
 create mode 100755 t/t6102-rev-list-unexpected-objects.sh

Range-diff against v1:
1:  2104508e42 = 1:  f09c374557 t: move 'hex2oct' into test-lib-functions.sh
2:  909cbcd4a1 ! 2:  137e2df24d t: introduce tests for unexpected object types
    @@ -20,21 +20,19 @@
         cases that are not broken (i.e., they have NULL-ness checks or similar),
         mark these as expecting success.

    -    Let A be the object referenced with an unexpected type, and B be the
    -    object doing the referencing. Do the following:
    -
    -      - test 'git rev-list --objects A B'. This causes A to be "cached", and
    -        presents the above scenario.
    -
    -    Likewise, if we have a tree entry that claims to be a tree (for example)
    -    but points to another object type (say, a blob), there are two ways we
    -    might find out:
    +    We might hit an unexpected type in two different ways (imagine we have a
    +    tree entry that claims to be a tree but actually points to a blob):

           - when we call lookup_tree(), we might find that we've already seen
    -        the object referenced as another type, in which case we'd get NULL
    +        the object referenced as a blob, in which case we'd get NULL. We
    +        can exercise this with "git rev-list --objects $blob $tree", which
    +        guarantees that the blob will have been parsed before we look in
    +        the tree. These tests are marked as "seen" in the test script.

           - we call lookup_tree() successfully, but when we try to read the
    -        object, we find out it's something else.
    +        object, we find out it's something else. We construct our tests
    +        such that $blob is not otherwise mentioned in $tree. These tests
    +        are marked as "lone" in the script.

         We should check that we behave sensibly in both cases (especially
         because it is easy for a malicious actor to provoke one case or the
    @@ -58,7 +56,8 @@
     +test_expect_success 'setup well-formed objects' '
     +	blob="$(printf "foo" | git hash-object -w --stdin)" &&
     +	tree="$(printf "100644 blob $blob\tfoo" | git mktree)" &&
    -+	commit="$(git commit-tree $tree -m "first commit")"
    ++	commit="$(git commit-tree $tree -m "first commit")" &&
    ++	git cat-file commit $commit >good-commit
     +'
     +
     +test_expect_success 'setup unexpected non-blob entry' '
    @@ -84,12 +83,11 @@
     +'
     +
     +test_expect_failure 'traverse unexpected non-tree entry (seen)' '
    -+	test_must_fail git rev-list --objects $blob $broken_tree >output 2>&1
    ++	test_must_fail git rev-list --objects $blob $broken_tree
     +'
     +
     +test_expect_success 'setup unexpected non-commit parent' '
    -+	git cat-file commit $commit |
    -+		perl -lpe "/^author/ && print q(parent $blob)" \
    ++	sed "/^author/ { h; s/.*/parent $blob/; G; }" <good-commit \
     +		>broken-commit &&
     +	broken_commit="$(git hash-object -w --literally -t commit \
     +		broken-commit)"
    @@ -107,8 +105,7 @@
     +'
     +
     +test_expect_success 'setup unexpected non-tree root' '
    -+	git cat-file commit $commit |
    -+	sed -e "s/$tree/$blob/" >broken-commit &&
    ++	sed -e "s/$tree/$blob/" <good-commit >broken-commit &&
     +	broken_commit="$(git hash-object -w --literally -t commit \
     +		broken-commit)"
     +'
    @@ -123,8 +120,9 @@
     +
     +test_expect_success 'setup unexpected non-commit tag' '
     +	git tag -a -m "tagged commit" tag $commit &&
    ++	git cat-file tag tag >good-tag &&
     +	test_when_finished "git tag -d tag" &&
    -+	git cat-file -p tag | sed -e "s/$commit/$blob/" >broken-tag &&
    ++	sed -e "s/$commit/$blob/" <good-tag >broken-tag &&
     +	tag=$(git hash-object -w --literally -t tag broken-tag)
     +'
     +
    @@ -139,9 +137,9 @@
     +
     +test_expect_success 'setup unexpected non-tree tag' '
     +	git tag -a -m "tagged tree" tag $tree &&
    ++	git cat-file tag tag >good-tag &&
     +	test_when_finished "git tag -d tag" &&
    -+	git cat-file -p tag |
    -+	sed -e "s/$tree/$blob/" >broken-tag &&
    ++	sed -e "s/$tree/$blob/" <good-tag >broken-tag &&
     +	tag=$(git hash-object -w --literally -t tag broken-tag)
     +'
     +
    @@ -156,9 +154,9 @@
     +
     +test_expect_success 'setup unexpected non-blob tag' '
     +	git tag -a -m "tagged blob" tag $blob &&
    ++	git cat-file tag tag >good-tag &&
     +	test_when_finished "git tag -d tag" &&
    -+	git cat-file -p tag |
    -+	sed -e "s/$blob/$commit/" >broken-tag &&
    ++	sed -e "s/$blob/$commit/" <good-tag >broken-tag &&
     +	tag=$(git hash-object -w --literally -t tag broken-tag)
     +'
     +
3:  42f1b5d5fd = 3:  1d1ac9b7a7 list-objects.c: handle unexpected non-blob entries
4:  62c9a6f2e0 ! 4:  97c7e23ec9 list-objects.c: handle unexpected non-tree entries
    @@ -31,7 +31,7 @@
      '

     -test_expect_failure 'traverse unexpected non-tree entry (seen)' '
    --	test_must_fail git rev-list --objects $blob $broken_tree >output 2>&1
    +-	test_must_fail git rev-list --objects $blob $broken_tree
     +test_expect_success 'traverse unexpected non-tree entry (seen)' '
     +	test_must_fail git rev-list --objects $blob $broken_tree >output 2>&1 &&
     +	test_i18ngrep "is not a tree" output
5:  d0ae70542d = 5:  e9400a9f77 get_commit_tree(): return NULL for broken tree
6:  a0c23c8c16 = 6:  88ca5dfe68 rev-list: let traversal die when --missing is not in use
7:  2a25796147 = 7:  e0bd479e82 rev-list: detect broken root trees
--
2.21.0.203.g358da99528

  parent reply	other threads:[~2019-04-10  2:13 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-05  3:37 [PATCH 0/7] harden unexpected object types checks Taylor Blau
2019-04-05  3:37 ` [PATCH 1/7] t: move 'hex2oct' into test-lib-functions.sh Taylor Blau
2019-04-05  3:37 ` [PATCH 2/7] t: introduce tests for unexpected object types Taylor Blau
2019-04-05 10:50   ` SZEDER Gábor
2019-04-05 18:24     ` Jeff King
2019-04-05 18:42       ` SZEDER Gábor
2019-04-05 18:52         ` Jeff King
2019-04-07 21:00           ` Ævar Arnfjörð Bjarmason
2019-04-09  2:29             ` Taylor Blau
2019-04-09  9:14               ` Ævar Arnfjörð Bjarmason
2019-04-10  1:59                 ` Taylor Blau
2019-04-08  5:27           ` Junio C Hamano
2019-04-05 19:25       ` Eric Sunshine
2019-04-05 20:53         ` Jeff King
2019-04-06  5:33           ` Taylor Blau
2019-04-08  6:44         ` Junio C Hamano
2019-04-09  2:30           ` Taylor Blau
2019-04-09  3:28             ` Eric Sunshine
2019-04-09  5:08               ` Taylor Blau
2019-04-09  8:02                 ` Eric Sunshine
2019-04-10  1:54                   ` Taylor Blau
2019-04-06  5:31       ` Taylor Blau
2019-04-05 18:31   ` Jeff King
2019-04-06  5:23     ` Taylor Blau
2019-04-05  3:37 ` [PATCH 3/7] list-objects.c: handle unexpected non-blob entries Taylor Blau
2019-04-05  3:37 ` [PATCH 4/7] list-objects.c: handle unexpected non-tree entries Taylor Blau
2019-04-05  3:37 ` [PATCH 5/7] get_commit_tree(): return NULL for broken tree Taylor Blau
2019-04-05  3:37 ` [PATCH 6/7] rev-list: let traversal die when --missing is not in use Taylor Blau
2019-04-05 18:41   ` Jeff King
2019-04-06  5:36     ` Taylor Blau
2019-04-07 13:41       ` Jeff King
2019-04-09  2:11         ` Taylor Blau
2019-04-05  3:37 ` [PATCH 7/7] rev-list: detect broken root trees Taylor Blau
2019-04-10  2:13 ` Taylor Blau [this message]
2019-04-10  2:13   ` [PATCH v2 1/7] t: move 'hex2oct' into test-lib-functions.sh Taylor Blau
2019-04-10  2:13   ` [PATCH v2 2/7] t: introduce tests for unexpected object types Taylor Blau
2019-04-10  2:13   ` [PATCH v2 3/7] list-objects.c: handle unexpected non-blob entries Taylor Blau
2019-04-10  2:13   ` [PATCH v2 4/7] list-objects.c: handle unexpected non-tree entries Taylor Blau
2019-04-10  2:13   ` [PATCH v2 5/7] get_commit_tree(): return NULL for broken tree Taylor Blau
2019-04-10  2:13   ` [PATCH v2 6/7] rev-list: let traversal die when --missing is not in use Taylor Blau
2019-04-10  2:13   ` [PATCH v2 7/7] rev-list: detect broken root trees Taylor Blau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1554861974.git.me@ttaylorr.com \
    --to=me@ttaylorr.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=sunshine@sunshineco.com \
    --cc=szeder.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).