git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Sixt <j6t@kdbg.org>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	Jacek Wielemborek <d33tah@gmail.com>
Subject: Re: [PATCH 1/3] t5313: test bounds-checks of corrupted/malicious pack/idx files
Date: Thu, 25 Feb 2016 20:12:58 +0100	[thread overview]
Message-ID: <56CF523A.8050208@kdbg.org> (raw)
In-Reply-To: <20160225142112.GA17811@sigill.intra.peff.net>

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?

---- 8< ----
From: Johannes Sixt <j6t@kdbg.org>
Subject: [PATCH] tests: overwrite bytes in files using a perl script instead of dd

The dd in my build environment on Windows crashes unpredictably. Work it
around by rewriting most instances with a helper function that uses perl
behind the scenes.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 t/t1060-object-corruption.sh          |  2 +-
 t/t5300-pack-object.sh                |  8 ++++----
 t/t5302-pack-index.sh                 |  5 +++--
 t/t5303-pack-corruption-resilience.sh |  2 +-
 t/test-lib-functions.sh               | 16 ++++++++++++++++
 5 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/t/t1060-object-corruption.sh b/t/t1060-object-corruption.sh
index 3f87051..e3c5de8 100755
--- a/t/t1060-object-corruption.sh
+++ b/t/t1060-object-corruption.sh
@@ -12,7 +12,7 @@ obj_to_file() {
 corrupt_byte() {
 	obj_file=$(obj_to_file "$1") &&
 	chmod +w "$obj_file" &&
-	printf '\0' | dd of="$obj_file" bs=1 seek="$2" conv=notrunc
+	printf '\0' | test_overwrite_bytes "$obj_file" "$2"
 }
 
 test_expect_success 'setup corrupt repo' '
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index fc2be63..f45a101 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -226,7 +226,7 @@ test_expect_success \
 test_expect_success \
     'verify-pack catches a corrupted pack signature' \
     'cat test-1-${packname_1}.pack >test-3.pack &&
-     echo | dd of=test-3.pack count=1 bs=1 conv=notrunc seek=2 &&
+     echo | test_overwrite_bytes test-3.pack 2 &&
      if git verify-pack test-3.idx
      then false
      else :;
@@ -235,7 +235,7 @@ test_expect_success \
 test_expect_success \
     'verify-pack catches a corrupted pack version' \
     'cat test-1-${packname_1}.pack >test-3.pack &&
-     echo | dd of=test-3.pack count=1 bs=1 conv=notrunc seek=7 &&
+     echo | test_overwrite_bytes test-3.pack 7 &&
      if git verify-pack test-3.idx
      then false
      else :;
@@ -244,7 +244,7 @@ test_expect_success \
 test_expect_success \
     'verify-pack catches a corrupted type/size of the 1st packed object data' \
     'cat test-1-${packname_1}.pack >test-3.pack &&
-     echo | dd of=test-3.pack count=1 bs=1 conv=notrunc seek=12 &&
+     echo | test_overwrite_bytes test-3.pack 12 &&
      if git verify-pack test-3.idx
      then false
      else :;
@@ -255,7 +255,7 @@ test_expect_success \
     'l=$(wc -c <test-3.idx) &&
      l=$(expr $l - 20) &&
      cat test-1-${packname_1}.pack >test-3.pack &&
-     printf "%20s" "" | dd of=test-3.idx count=20 bs=1 conv=notrunc seek=$l &&
+     printf "%20s" "" | test_overwrite_bytes test-3.idx $l &&
      if git verify-pack test-3.pack
      then false
      else :;
diff --git a/t/t5302-pack-index.sh b/t/t5302-pack-index.sh
index c2fc584..5a82f19 100755
--- a/t/t5302-pack-index.sh
+++ b/t/t5302-pack-index.sh
@@ -225,8 +225,9 @@ test_expect_success \
      obj=$(git hash-object file_001) &&
      nr=$(index_obj_nr ".git/objects/pack/pack-${pack1}.idx" $obj) &&
      chmod +w ".git/objects/pack/pack-${pack1}.idx" &&
-     printf xxxx | dd of=".git/objects/pack/pack-${pack1}.idx" conv=notrunc \
-        bs=1 count=4 seek=$((8 + 256 * 4 + $(wc -l <obj-list) * 20 + $nr * 4)) &&
+     printf xxxx |
+		test_overwrite_bytes ".git/objects/pack/pack-${pack1}.idx" \
+			$((8 + 256 * 4 + $(wc -l <obj-list) * 20 + $nr * 4)) &&
      ( while read obj
        do git cat-file -p $obj >/dev/null || exit 1
        done <obj-list ) &&
diff --git a/t/t5303-pack-corruption-resilience.sh b/t/t5303-pack-corruption-resilience.sh
index 5940ce2..9d2e437 100755
--- a/t/t5303-pack-corruption-resilience.sh
+++ b/t/t5303-pack-corruption-resilience.sh
@@ -51,7 +51,7 @@ do_corrupt_object() {
     ofs=$(git show-index < ${pack}.idx | grep $1 | cut -f1 -d" ") &&
     ofs=$(($ofs + $2)) &&
     chmod +w ${pack}.pack &&
-    dd of=${pack}.pack bs=1 conv=notrunc seek=$ofs &&
+    test_overwrite_bytes "${pack}.pack" "$ofs" &&
     test_must_fail git verify-pack ${pack}.pack
 }
 
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index ec6125d..f7ba047 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -880,6 +880,22 @@ test_skip_or_die () {
 	esac
 }
 
+# Overwrite bytes at an offset in a file
+# $1 ... the file to modify
+# $2 ... byte offset into file
+# stdin ... new bytes
+test_overwrite_bytes () {
+	perl -e '
+		$fname = shift @ARGV;
+		$offset = shift @ARGV;
+		$bytes = <>;
+		open my $fh, "+<", $fname	or die "open $fname: $!\n";
+		seek($fh, $offset, 0)		or die "seek $fname: $!\n";
+		syswrite($fh, $bytes)		or die "write $fname: $!\n";
+		close $fh			or die "close $fname: $!\n";
+	' "$@"
+}
+
 # The following mingw_* functions obey POSIX shell syntax, but are actually
 # bash scripts, and are meant to be used only with bash on Windows.
 
-- 
2.7.0.118.g90056ae

  reply	other threads:[~2016-02-25 19:13 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-25 14:20 [PATCH 0/3] out-of-bounds access from corrupted .idx files Jeff King
2016-02-25 14:21 ` [PATCH 1/3] t5313: test bounds-checks of corrupted/malicious pack/idx files Jeff King
2016-02-25 19:12   ` Johannes Sixt [this message]
2016-02-25 20:31     ` Junio C Hamano
2016-02-25 22:07       ` Jeff King
2016-02-25 14:22 ` [PATCH 2/3] nth_packed_object_offset: bounds-check extended offset Jeff King
2016-02-25 14:23 ` [PATCH 3/3] use_pack: handle signed off_t overflow Jeff King
2016-02-27  7:49 ` [PATCH 4/3] sha1_file.c: mark strings for translation Nguyễn Thái Ngọc Duy
2016-02-27 17:41   ` Junio C Hamano
2016-02-27 18:25     ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56CF523A.8050208@kdbg.org \
    --to=j6t@kdbg.org \
    --cc=d33tah@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).