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

[Eric Sunshine]

On Wed, Sep 5, 2018 at 10:46 AM Derrick Stolee via GitGitGadget
 wrote:
> Add tests that check corrupted headers in a few ways:
> [...]
> Signed-off-by: Derrick Stolee 
> ---
> diff --git 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' '
> +corrupt_midx_and_verify() {
> +   POS=$1
> +   DATA="${2:-\0}"
> +   OBJDIR=$3
> +   GREPSTR="$4"

If you happen to re-roll, perhaps make these assignments part of the
&&-chain to protect against someone coming along some day an inserting
code before them without realizing that the &&-chain is broken.

> +   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
> +}

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

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee 

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 
---
 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..836d2bdb53 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
+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   []
 corrupt_data () {
file=$1
-- 
gitgitgadget