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

[PATCH v2 02/11] multi-pack-index: verify bad header

From
Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Date
Sep 13, 2018, 18:02 UTC
Message-ID
<95908958302a4a27d8ba444ed7bfd867cb43cd20.1536861730.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.34.v2.git.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>

When verifying if a multi-pack-index file is valid, we want the command to fail to signal an invalid file. Previously, we wrote an error to stderr and continued as if we had no multi-pack-index. Now, die() instead of error().

Add tests that check corrupted headers in a few ways:
* Bad signature
* Bad file version
* Bad hash version
* Truncated hash count
* Extended hash count
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 midx.c                      | 18 +++++----------
 t/t5319-multi-pack-index.sh | 46 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 51 insertions(+), 13 deletions(-)
diff --git a/midx.c b/midx.c
index b253bed517..ec78254bb6 100644
--- a/midx.c
+++ b/midx.c
@@ -76,24 +76,18 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local
 	m->local = local;
 
 	m->signature = get_be32(m->data);
-	if (m->signature != MIDX_SIGNATURE) {
-		error(_("multi-pack-index signature 0x%08x does not match signature 0x%08x"),
+	if (m->signature != MIDX_SIGNATURE)
+		die(_("multi-pack-index signature 0x%08x does not match signature 0x%08x"),
 		      m->signature, MIDX_SIGNATURE);
-		goto cleanup_fail;
-	}
 
 	m->version = m->data[MIDX_BYTE_FILE_VERSION];
-	if (m->version != MIDX_VERSION) {
-		error(_("multi-pack-index version %d not recognized"),
+	if (m->version != MIDX_VERSION)
+		die(_("multi-pack-index version %d not recognized"),
 		      m->version);
-		goto cleanup_fail;
-	}
 
 	hash_version = m->data[MIDX_BYTE_HASH_VERSION];
-	if (hash_version != MIDX_HASH_VERSION) {
-		error(_("hash version %u does not match"), hash_version);
-		goto cleanup_fail;
-	}
+	if (hash_version != MIDX_HASH_VERSION)
+		die(_("hash version %u does not match"), hash_version);
 	m->hash_len = MIDX_HASH_LEN;
 
 	m->num_chunks = m->data[MIDX_BYTE_NUM_CHUNKS];
diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index 1c4e0e6d31..e04b5f43a2 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -154,6 +154,51 @@ test_expect_success 'verify multi-pack-index success' '
 	git multi-pack-index verify --object-dir=$objdir
 '
 
+# usage: corrupt_midx_and_verify <pos> <data> <objdir> <string>
+corrupt_midx_and_verify() {
+	POS=$1 &&
+	DATA="${2:-\0}" &&
+	OBJDIR=$3 &&
+	GREPSTR="$4" &&
+	FILE=$OBJDIR/pack/multi-pack-index &&
+	chmod a+w $FILE &&
+	test_when_finished mv midx-backup $FILE &&
+	cp $FILE midx-backup &&
+	printf "$DATA" | dd of="$FILE" bs=1 seek="$POS" conv=notrunc &&
+	test_must_fail git multi-pack-index verify --object-dir=$OBJDIR 2>test_err &&
+	grep -v "^+" test_err >err &&
+	test_i18ngrep "$GREPSTR" err
+}
+
+test_expect_success 'verify bad signature' '
+	corrupt_midx_and_verify 0 "\00" $objdir \
+		"multi-pack-index signature"
+'
+
+MIDX_BYTE_VERSION=4
+MIDX_BYTE_OID_VERSION=5
+MIDX_BYTE_CHUNK_COUNT=6
+
+test_expect_success 'verify bad version' '
+	corrupt_midx_and_verify $MIDX_BYTE_VERSION "\00" $objdir \
+		"multi-pack-index version"
+'
+
+test_expect_success 'verify bad OID version' '
+	corrupt_midx_and_verify $MIDX_BYTE_OID_VERSION "\02" $objdir \
+		"hash version"
+'
+
+test_expect_success 'verify truncated chunk count' '
+	corrupt_midx_and_verify $MIDX_BYTE_CHUNK_COUNT "\01" $objdir \
+		"missing required"
+'
+
+test_expect_success 'verify extended chunk count' '
+	corrupt_midx_and_verify $MIDX_BYTE_CHUNK_COUNT "\07" $objdir \
+		"terminating multi-pack-index chunk id appears earlier than expected"
+'
+
 test_expect_success 'repack removes multi-pack-index' '
 	test_path_is_file $objdir/pack/multi-pack-index &&
 	git repack -adf &&
@@ -191,7 +236,6 @@ test_expect_success 'multi-pack-index in an alternate' '
 
 compare_results_with_midx "with alternate (remote midx)"
 
-
 # usage: corrupt_data <file> <pos> [<data>]
 corrupt_data () {
 	file=$1
-- 
gitgitgadget
Previous: Derrick Stolee via GitGitGadgetNext: Derrick Stolee via GitGitGadget
Message 23 of 34 in “Add 'git multi-pack-index verify' command”
  1. 00/11 Add 'git multi-pack-index verify' commandDerrick Stolee via GitGitGadget, Sep 5, 2018
  2. 01/11 multi-pack-index: add 'verify' verbDerrick Stolee via GitGitGadget, Sep 5, 2018
  3. Eric SunshineSep 5, 2018
  4. Derrick StoleeSep 5, 2018
  5. 02/11 multi-pack-index: verify bad headerDerrick Stolee via GitGitGadget, Sep 5, 2018
  6. Eric SunshineSep 7, 2018
  7. 04/11 multi-pack-index: verify packname orderDerrick Stolee via GitGitGadget, Sep 5, 2018
  8. Stefan BellerSep 5, 2018
  9. Derrick StoleeSep 5, 2018
  10. Stefan BellerSep 5, 2018
  11. Derrick StoleeSep 5, 2018
  12. 05/11 multi-pack-index: verify missing packDerrick Stolee via GitGitGadget, Sep 5, 2018
  13. 06/11 multi-pack-index: verify oid fanout orderDerrick Stolee via GitGitGadget, Sep 5, 2018
  14. 07/11 multi-pack-index: verify oid lookup orderDerrick Stolee via GitGitGadget, Sep 5, 2018
  15. 08/11 multi-pack-index: fix 32-bit vs 64-bit size checkDerrick Stolee via GitGitGadget, Sep 5, 2018
  16. 03/11 multi-pack-index: verify corrupt chunk lookup tableDerrick Stolee via GitGitGadget, Sep 5, 2018
  17. 09/11 multi-pack-index: verify object offsetsDerrick Stolee via GitGitGadget, Sep 5, 2018
  18. Eric SunshineSep 7, 2018
  19. Derrick StoleeSep 7, 2018
  20. 10/11 multi-pack-index: report progress during 'verify'Derrick Stolee via GitGitGadget, Sep 5, 2018
  21. 11/11 fsck: verify multi-pack-indexDerrick Stolee via GitGitGadget, Sep 5, 2018
  22. 00/11 Add 'git multi-pack-index verify' commandDerrick Stolee via GitGitGadget, Sep 13, 2018
  23. 02/11 multi-pack-index: verify bad headerDerrick Stolee via GitGitGadget, Sep 13, 2018
  24. 03/11 multi-pack-index: verify corrupt chunk lookup tableDerrick Stolee via GitGitGadget, Sep 13, 2018
  25. 04/11 multi-pack-index: verify packname orderDerrick Stolee via GitGitGadget, Sep 13, 2018
  26. 05/11 multi-pack-index: verify missing packDerrick Stolee via GitGitGadget, Sep 13, 2018
  27. 06/11 multi-pack-index: verify oid fanout orderDerrick Stolee via GitGitGadget, Sep 13, 2018
  28. 07/11 multi-pack-index: verify oid lookup orderDerrick Stolee via GitGitGadget, Sep 13, 2018
  29. 08/11 multi-pack-index: fix 32-bit vs 64-bit size checkDerrick Stolee via GitGitGadget, Sep 13, 2018
  30. 09/11 multi-pack-index: verify object offsetsDerrick Stolee via GitGitGadget, Sep 13, 2018
  31. 10/11 multi-pack-index: report progress during 'verify'Derrick Stolee via GitGitGadget, Sep 13, 2018
  32. Ævar Arnfjörð BjarmasonSep 13, 2018
  33. 11/11 fsck: verify multi-pack-indexDerrick Stolee via GitGitGadget, Sep 13, 2018
  34. 01/11 multi-pack-index: add 'verify' verbDerrick Stolee via GitGitGadget, Sep 13, 2018

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.