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
