All of lore.kernel.org
 help / color / mirror / 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 2/7] t: introduce tests for unexpected object types
Date: Tue, 9 Apr 2019 19:13:14 -0700	[thread overview]
Message-ID: <137e2df24d80f796675691e57d08948b6eede6e3.1554861974.git.me@ttaylorr.com> (raw)
In-Reply-To: <cover.1554861974.git.me@ttaylorr.com>

Call an object's type "unexpected" when the actual type of an object
does not match Git's contextual expectation. For example, a tree entry
whose mode differs from the object's actual type, or a commit's parent
which is not another commit, and so on.

This can manifest itself in various unfortunate ways, including Git
SIGSEGV-ing under specific conditions. Consider the following example:
Git traverses a blob (say, via `git rev-list`), and then tries to read
out a tree-entry which lists that object as something other than a blob.
In this case, `lookup_blob()` will return NULL, and the subsequent
dereference will result in a SIGSEGV.

Introduce tests that present objects of "unexpected" type in the above
fashion to 'git rev-list'. Mark as failures the combinations that are
already broken (i.e., they exhibit the segfault described above). In the
cases that are not broken (i.e., they have NULL-ness checks or similar),
mark these as expecting success.

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 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. 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
other).

Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Jeff King <peff@peff.net>
---
 t/t6102-rev-list-unexpected-objects.sh | 123 +++++++++++++++++++++++++
 1 file changed, 123 insertions(+)
 create mode 100755 t/t6102-rev-list-unexpected-objects.sh

diff --git a/t/t6102-rev-list-unexpected-objects.sh b/t/t6102-rev-list-unexpected-objects.sh
new file mode 100755
index 0000000000..15072ecce3
--- /dev/null
+++ b/t/t6102-rev-list-unexpected-objects.sh
@@ -0,0 +1,123 @@
+#!/bin/sh
+
+test_description='git rev-list should handle unexpected object types'
+
+. ./test-lib.sh
+
+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")" &&
+	git cat-file commit $commit >good-commit
+'
+
+test_expect_success 'setup unexpected non-blob entry' '
+	printf "100644 foo\0$(echo $tree | hex2oct)" >broken-tree &&
+	broken_tree="$(git hash-object -w --literally -t tree broken-tree)"
+'
+
+test_expect_failure 'traverse unexpected non-blob entry (lone)' '
+	test_must_fail git rev-list --objects $broken_tree
+'
+
+test_expect_failure 'traverse unexpected non-blob entry (seen)' '
+	test_must_fail git rev-list --objects $tree $broken_tree
+'
+
+test_expect_success 'setup unexpected non-tree entry' '
+	printf "40000 foo\0$(echo $blob | hex2oct)" >broken-tree &&
+	broken_tree="$(git hash-object -w --literally -t tree broken-tree)"
+'
+
+test_expect_failure 'traverse unexpected non-tree entry (lone)' '
+	test_must_fail git rev-list --objects $broken_tree
+'
+
+test_expect_failure 'traverse unexpected non-tree entry (seen)' '
+	test_must_fail git rev-list --objects $blob $broken_tree
+'
+
+test_expect_success 'setup unexpected non-commit parent' '
+	sed "/^author/ { h; s/.*/parent $blob/; G; }" <good-commit \
+		>broken-commit &&
+	broken_commit="$(git hash-object -w --literally -t commit \
+		broken-commit)"
+'
+
+test_expect_success 'traverse unexpected non-commit parent (lone)' '
+	test_must_fail git rev-list --objects $broken_commit >output 2>&1 &&
+	test_i18ngrep "not a commit" output
+'
+
+test_expect_success 'traverse unexpected non-commit parent (seen)' '
+	test_must_fail git rev-list --objects $commit $broken_commit \
+		>output 2>&1 &&
+	test_i18ngrep "not a commit" output
+'
+
+test_expect_success 'setup unexpected non-tree root' '
+	sed -e "s/$tree/$blob/" <good-commit >broken-commit &&
+	broken_commit="$(git hash-object -w --literally -t commit \
+		broken-commit)"
+'
+
+test_expect_failure 'traverse unexpected non-tree root (lone)' '
+	test_must_fail git rev-list --objects $broken_commit
+'
+
+test_expect_failure 'traverse unexpected non-tree root (seen)' '
+	test_must_fail git rev-list --objects $blob $broken_commit
+'
+
+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" &&
+	sed -e "s/$commit/$blob/" <good-tag >broken-tag &&
+	tag=$(git hash-object -w --literally -t tag broken-tag)
+'
+
+test_expect_success 'traverse unexpected non-commit tag (lone)' '
+	test_must_fail git rev-list --objects $tag
+'
+
+test_expect_success 'traverse unexpected non-commit tag (seen)' '
+	test_must_fail git rev-list --objects $blob $tag >output 2>&1 &&
+	test_i18ngrep "not a commit" output
+'
+
+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" &&
+	sed -e "s/$tree/$blob/" <good-tag >broken-tag &&
+	tag=$(git hash-object -w --literally -t tag broken-tag)
+'
+
+test_expect_success 'traverse unexpected non-tree tag (lone)' '
+	test_must_fail git rev-list --objects $tag
+'
+
+test_expect_success 'traverse unexpected non-tree tag (seen)' '
+	test_must_fail git rev-list --objects $blob $tag >output 2>&1 &&
+	test_i18ngrep "not a tree" output
+'
+
+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" &&
+	sed -e "s/$blob/$commit/" <good-tag >broken-tag &&
+	tag=$(git hash-object -w --literally -t tag broken-tag)
+'
+
+test_expect_failure 'traverse unexpected non-blob tag (lone)' '
+	test_must_fail git rev-list --objects $tag
+'
+
+test_expect_success 'traverse unexpected non-blob tag (seen)' '
+	test_must_fail git rev-list --objects $commit $tag >output 2>&1 &&
+	test_i18ngrep "not a blob" output
+'
+
+test_done
-- 
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 ` [PATCH v2 0/7] harden unexpected object types checks Taylor Blau
2019-04-10  2:13   ` [PATCH v2 1/7] t: move 'hex2oct' into test-lib-functions.sh Taylor Blau
2019-04-10  2:13   ` Taylor Blau [this message]
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

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

  git send-email \
    --in-reply-to=137e2df24d80f796675691e57d08948b6eede6e3.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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.