Re: [PATCH 3/3] t5303: add tests for corrupted deltas

[Jeff King <peff@peff.net>]

On Wed, Aug 29, 2018 at 06:03:53PM -0400, Jeff King wrote:

> Without your second patch applied, this complains about mmap-ing
> /dev/null (or any zero-length file).
> 
> Also, \x escapes are sadly not portable (dash, for example, does not
> respect them). You have to use octal instead (which is not too onerous
> for these small numbers).
> 
> I needed the patch below to get it to behave as expected (I also
> annotated the deltas to make it more comprehensible to somebody who
> hasn't just been digging in the patch code ;) ).
> 
> I wonder if we should more fully test the 4 cases I outlined in my
> earlier mail, too.

So here's a version which checks each of those cases (plus your minimal
base-case and the trailing garbage case from your original).

I did it as a straight patch rather than on top of yours, since I think
that's easier to read (and I'd expect us to squash together whatever we
end up with anyway).

This doesn't cover out-of-bounds reads while parsing the offset/size.
It's dinner-time here, but I may take a look at that later tonight.

---
diff --git a/t/t5303-pack-corruption-resilience.sh b/t/t5303-pack-corruption-resilience.sh
index 3634e258f8..305922eeb3 100755
--- a/t/t5303-pack-corruption-resilience.sh
+++ b/t/t5303-pack-corruption-resilience.sh
@@ -311,4 +311,81 @@ test_expect_success \
      test_must_fail git cat-file blob $blob_2 > /dev/null &&
      test_must_fail git cat-file blob $blob_3 > /dev/null'
 
+# Base sanity check; this is the smallest possible delta.
+#
+# \5 - five bytes in base (though we do not use it)
+# \1 - one byte in result
+# \1 - copy one byte (X)
+test_expect_success \
+    'apply good minimal delta' \
+    'printf "\5\1\1X" > minimal_delta &&
+     echo base >base &&
+     test-tool delta -p base minimal_delta /dev/null
+     '
+
+# This delta has too many literal bytes to fit in destination.
+#
+# \5 - five bytes in base (though we do not use it)
+# \1 - 1 byte in result
+# \2 - copy two bytes (one too many)
+test_expect_success \
+    'apply truncated delta' \
+    'printf "\5\1\2XX" > too_big_literal &&
+     echo base >base &&
+     test_must_fail test-tool delta -p base too_big_literal /dev/null
+     '
+
+# This delta has too many copy bytes to fit in destination.
+#
+# \5 - five bytes in base
+# \1 - one byte in result
+# \221 - copy, one byte offset, one byte size
+#   \0 - copy from offset 0
+#   \2 - copy two bytes (one too many)
+test_expect_success \
+    'apply delta with trailing garbage command' \
+    'printf "\5\1\221\0\2" > too_big_copy &&
+     echo base >base &&
+     test_must_fail test-tool delta -p base too_big_copy /dev/null
+     '
+
+# This delta has too few bytes in the delta itself.
+#
+# \5 - five bytes in base (though we do not use it)
+# \2 - two bytes in result
+# \2 - copy two bytes (we are short one)
+test_expect_success \
+    'apply truncated delta' \
+    'printf "\5\2\2X" > truncated_delta &&
+     echo base >base &&
+     test_must_fail test-tool delta -p base truncated_delta /dev/null
+     '
+
+# This delta has too few bytes in the base.
+#
+# \5 - five bytes in base
+# \6 - six bytes in result
+# \221 - copy, one byte offset, one byte size
+#   \0 - copy from offset 0
+#   \6 - copy six bytes (one too many)
+test_expect_success \
+    'apply delta with trailing garbage command' \
+    'printf "\5\6\221\0\6" > truncated_base &&
+     echo base >base &&
+     test_must_fail test-tool delta -p base truncated_base /dev/null
+     '
+
+# Trailing garbage command.
+#
+# \5 - five bytes in base (though we do not use it)
+# \1 - one byte in result
+# \1 - copy one byte (X)
+# \1 - trailing garbage command
+test_expect_success \
+    'apply delta with trailing garbage command' \
+    'printf "\5\1\1X\1" > tail_garbage_delta &&
+     echo base >base &&
+     test_must_fail test-tool delta -p base tail_garbage_delta /dev/null
+     '
+
 test_done