Re: [PATCH 1/3] t5313: test bounds-checks of corrupted/malicious pack/idx files

[Jeff King <peff@peff.net>]

On Thu, Feb 25, 2016 at 12:31:12PM -0800, Junio C Hamano wrote:

> Johannes Sixt <j6t@kdbg.org> writes:
> 
> > Am 25.02.2016 um 15:21 schrieb Jeff King:
> >> +munge () {
> >> +	printf "$3" | dd of="$1" bs=1 conv=notrunc seek=$2
> >> +}
> >
> > Instead of adding another call of dd, would it be an option to insert
> > the following patch at the front of this series and then use
> > test_overwrite_bytes?
> 
> It would be an option, but I'd want to merge this to 'next' today
> without waiting for a reroll.  Change from dd to custom script can
> be done as a follow-up topic after the dust settles, if necessary.

That's fine with me, and in general I am fine with the conversion from
dd to perl. I have to look up the dd command line options each time
anyway (was it "skip" or "seek"...?). ;)

But note that the use of "dd" in the "bogus offset inside extended
table" test in my patch is different from the others. It copies a set
amount of bytes, and inserts some custom data, and then copies more.

I think you could replace the first with "head -c", and the latter with
"tail -c". In the latter you'd have to compute the distance from the end
rather than the seek offset, but we know that already: it's 20 + 20 for
the trailing sha1s. Like this, I think:

diff --git a/t/t5313-pack-bounds-checks.sh b/t/t5313-pack-bounds-checks.sh
index a8a587a..20bc0a3 100755
--- a/t/t5313-pack-bounds-checks.sh
+++ b/t/t5313-pack-bounds-checks.sh
@@ -144,10 +144,10 @@ test_expect_success 'bogus offset inside v2 extended table' '
 	# We have to make extra room for the table, so we cannot
 	# just munge in place as usual.
 	{
-		dd if=$idx bs=1 count=$(($(ofs_table 2) + 4)) &&
+		head -c $(($(ofs_table 2) + 4)) $idx &&
 		printf "\200\0\0\0" &&
 		printf "\377\0\0\0\0\0\0\0" &&
-		dd if=$idx bs=1 skip=$(extended_table 2)
+		tail -c 40 $idx
 	} >tmp &&
 	mv tmp "$idx" &&
 	clear_base &&