git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/7] t: introduce tests for unexpected object types

From
Taylor Blau <me@ttaylorr.com>
Date
Apr 5, 2019, 03:37 UTC
Message-ID
<ef6b4f380435d9743a56f47f68c18123bf0a0933.1554435033.git.me@ttaylorr.com>
In-Reply-To
<cover.1554435033.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.

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:

  - 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
  - we call lookup_tree() successfully, but when we try to read the
    object, we find out it's something else.

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..472b08528a
--- /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")"
+'
+
+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 >output 2>&1
+'
+
+test_expect_success 'setup unexpected non-commit parent' '
+	git cat-file commit $commit |
+		perl -lpe "/^author/ && print q(parent $blob)" \
+		>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' '
+	git cat-file commit $commit |
+	sed -e "s/$tree/$blob/" >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 &&
+	test_when_finished "git tag -d tag" &&
+	git cat-file -p tag | sed -e "s/$commit/$blob/" >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 &&
+	test_when_finished "git tag -d tag" &&
+	git cat-file -p tag |
+	sed -e "s/$tree/$blob/" >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 &&
+	test_when_finished "git tag -d tag" &&
+	git cat-file -p tag |
+	sed -e "s/$blob/$commit/" >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
Previous: Taylor BlauNext: SZEDER Gábor
Message 3 of 41 in “harden unexpected object types checks”
  1. 0/7 harden unexpected object types checksTaylor Blau, Apr 5, 2019
  2. 1/7 t: move 'hex2oct' into test-lib-functions.shTaylor Blau, Apr 5, 2019
  3. 2/7 t: introduce tests for unexpected object typesTaylor Blau, Apr 5, 2019
  4. SZEDER GáborApr 5, 2019
  5. Jeff KingApr 5, 2019
  6. SZEDER GáborApr 5, 2019
  7. Jeff KingApr 5, 2019
  8. Ævar Arnfjörð BjarmasonApr 7, 2019
  9. Taylor BlauApr 9, 2019
  10. Ævar Arnfjörð BjarmasonApr 9, 2019
  11. Taylor BlauApr 10, 2019
  12. Junio C HamanoApr 8, 2019
  13. Eric SunshineApr 5, 2019
  14. Jeff KingApr 5, 2019
  15. Taylor BlauApr 6, 2019
  16. Junio C HamanoApr 8, 2019
  17. Taylor BlauApr 9, 2019
  18. Eric SunshineApr 9, 2019
  19. Taylor BlauApr 9, 2019
  20. Eric SunshineApr 9, 2019
  21. Taylor BlauApr 10, 2019
  22. Taylor BlauApr 6, 2019
  23. Jeff KingApr 5, 2019
  24. Taylor BlauApr 6, 2019
  25. 3/7 list-objects.c: handle unexpected non-blob entriesTaylor Blau, Apr 5, 2019
  26. 4/7 list-objects.c: handle unexpected non-tree entriesTaylor Blau, Apr 5, 2019
  27. 5/7 get_commit_tree(): return NULL for broken treeTaylor Blau, Apr 5, 2019
  28. 6/7 rev-list: let traversal die when --missing is not in useTaylor Blau, Apr 5, 2019
  29. Jeff KingApr 5, 2019
  30. Taylor BlauApr 6, 2019
  31. Jeff KingApr 7, 2019
  32. Taylor BlauApr 9, 2019
  33. 7/7 rev-list: detect broken root treesTaylor Blau, Apr 5, 2019
  34. 0/7 harden unexpected object types checksTaylor Blau, Apr 10, 2019
  35. 1/7 t: move 'hex2oct' into test-lib-functions.shTaylor Blau, Apr 10, 2019
  36. 2/7 t: introduce tests for unexpected object typesTaylor Blau, Apr 10, 2019
  37. 3/7 list-objects.c: handle unexpected non-blob entriesTaylor Blau, Apr 10, 2019
  38. 4/7 list-objects.c: handle unexpected non-tree entriesTaylor Blau, Apr 10, 2019
  39. 5/7 get_commit_tree(): return NULL for broken treeTaylor Blau, Apr 10, 2019
  40. 6/7 rev-list: let traversal die when --missing is not in useTaylor Blau, Apr 10, 2019
  41. 7/7 rev-list: detect broken root treesTaylor Blau, Apr 10, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.