Re: [PATCH 1/3] t5313: test bounds-checks of corrupted/malicious pack/idx files
- From
Jeff King <peff@peff.net>
- Date
- Feb 25, 2016, 22:07 UTC
- Message-ID
- <20160225220724.GB10267@sigill.intra.peff.net>
- In-Reply-To
- <xmqq4mcwo7lb.fsf@gitster.mtv.corp.google.com>
On Thu, Feb 25, 2016 at 12:31:12PM -0800, Junio C Hamano wrote:
Show 14 quoted lines
> 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 &&