[PATCH 0/3] out-of-bounds access from corrupted .idx files

[PATCH 0/3] out-of-bounds access from corrupted .idx files

[Jeff King]

This series teaches Git to detect a few problems with corrupted .idx
files, and adds tests for some more cases.  There's conceptually some
overlap with t5300, but I don't think it was covering any of these cases
explicitly.

There are two real bugs that could cause segfaults or bus errors via
bogus reads (but never writes). On top of that, these are all problems
in .idx files, which are usually generated locally. So I don't think
there's anything particularly security interesting here. You'd need a
situation where you convince somebody to read your .idx files (so maybe
a multi-user server), and then I don't see how you'd turn it into remote
code execution.

I think with these patches, fuzzing .idx files should never result in
any memory problems (though of course git will die()).  Famous last
words, of course. I stopped short of poking at other file formats, which
might have similar issues. Obvious candidates are .bitmap files, and the
on-disk $GIT_DIR/index.

  [1/3]: t5313: test bounds-checks of corrupted/malicious pack/idx files
  [2/3]: nth_packed_object_offset: bounds-check extended offset
  [3/3]: use_pack: handle signed off_t overflow

-Peff

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

[Jeff King]

Our on-disk .pack and .idx files may reference other data by
offset. We should make sure that we are not fooled by
corrupt data into accessing memory outside of our mmap'd
boundaries.

This patch adds a series of tests for offsets found in .pack
and .idx files. For the most part we get this right, but
there are two tests of .idx files marked as failures: we do
not bounds-check offsets in the v2 index's extended offset
table, nor do we handle .idx offsets that overflow a signed
off_t.

With these tests, we should have good coverage of all
offsets found in these files. Note that this doesn't cover
.bitmap files, which may have similar bugs.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t5313-pack-bounds-checks.sh | 179 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 179 insertions(+)
 create mode 100755 t/t5313-pack-bounds-checks.sh

diff --git a/t/t5313-pack-bounds-checks.sh b/t/t5313-pack-bounds-checks.sh
new file mode 100755
index 0000000..efc5197
--- /dev/null
+++ b/t/t5313-pack-bounds-checks.sh
@@ -0,0 +1,179 @@
+#!/bin/sh
+
+test_description='bounds-checking of access to mmapped on-disk file formats'
+. ./test-lib.sh
+
+clear_base () {
+	test_when_finished 'restore_base' &&
+	rm -f $base
+}
+
+restore_base () {
+	cp base-backup/* .git/objects/pack/
+}
+
+do_pack () {
+	pack_objects=$1; shift
+	sha1=$(
+		for i in $pack_objects
+		do
+			echo $i
+		done | git pack-objects "$@" .git/objects/pack/pack
+	) &&
+	pack=.git/objects/pack/pack-$sha1.pack &&
+	idx=.git/objects/pack/pack-$sha1.idx &&
+	chmod +w $pack $idx &&
+	test_when_finished 'rm -f "$pack" "$idx"'
+}
+
+munge () {
+	printf "$3" | dd of="$1" bs=1 conv=notrunc seek=$2
+}
+
+# Offset in a v2 .idx to its initial and extended offset tables. For an index
+# with "nr" objects, this is:
+#
+#   magic(4) + version(4) + fan-out(4*256) + sha1s(20*nr) + crc(4*nr),
+#
+# for the initial, and another ofs(4*nr) past that for the extended.
+#
+ofs_table () {
+	echo $((4 + 4 + 4*256 + 20*$1 + 4*$1))
+}
+extended_table () {
+	echo $(($(ofs_table "$1") + 4*$1))
+}
+
+test_expect_success 'set up base packfile and variables' '
+	# the hash of this content starts with ff, which
+	# makes some later computations much simpler
+	echo 74 >file &&
+	git add file &&
+	git commit -m base &&
+	git repack -ad &&
+	base=$(echo .git/objects/pack/*) &&
+	chmod +w $base &&
+	mkdir base-backup &&
+	cp $base base-backup/ &&
+	object=$(git rev-parse HEAD:file)
+'
+
+test_expect_success 'pack/index object count mismatch' '
+	do_pack $object &&
+	munge $pack 8 "\377\0\0\0" &&
+	clear_base &&
+
+	# We enumerate the objects from the completely-fine
+	# .idx, but notice later that the .pack is bogus
+	# and fail to show any data.
+	echo "$object missing" >expect &&
+	git cat-file --batch-all-objects --batch-check >actual &&
+	test_cmp expect actual &&
+
+	# ...and here fail to load the object (without segfaulting),
+	# but fallback to a good copy if available.
+	test_must_fail git cat-file blob $object &&
+	restore_base &&
+	git cat-file blob $object >actual &&
+	test_cmp file actual &&
+
+	# ...and make sure that index-pack --verify, which has its
+	# own reading routines, does not segfault.
+	test_must_fail git index-pack --verify $pack
+'
+
+test_expect_success 'matched bogus object count' '
+	do_pack $object &&
+	munge $pack 8 "\377\0\0\0" &&
+	munge $idx $((255 * 4)) "\377\0\0\0" &&
+	clear_base &&
+
+	# Unlike above, we should notice early that the .idx is totally
+	# bogus, and not even enumerate its contents.
+	>expect &&
+	git cat-file --batch-all-objects --batch-check >actual &&
+	test_cmp expect actual &&
+
+	# But as before, we can do the same object-access checks.
+	test_must_fail git cat-file blob $object &&
+	restore_base &&
+	git cat-file blob $object >actual &&
+	test_cmp file actual &&
+
+	test_must_fail git index-pack --verify $pack
+'
+
+# Note that we cannot check the fallback case for these
+# further .idx tests, as we notice the problem in functions
+# whose interface doesn't allow an error return (like use_pack()),
+# and thus we just die().
+#
+# There's also no point in doing enumeration tests, as
+# we are munging offsets here, which are about looking up
+# specific objects.
+
+test_expect_success 'bogus object offset (v1)' '
+	do_pack $object --index-version=1 &&
+	munge $idx $((4 * 256)) "\377\0\0\0" &&
+	clear_base &&
+	test_must_fail git cat-file blob $object &&
+	test_must_fail git index-pack --verify $pack
+'
+
+test_expect_success 'bogus object offset (v2, no msb)' '
+	do_pack $object --index-version=2 &&
+	munge $idx $(ofs_table 1) "\0\377\0\0" &&
+	clear_base &&
+	test_must_fail git cat-file blob $object &&
+	test_must_fail git index-pack --verify $pack
+'
+
+test_expect_failure 'bogus offset into v2 extended table' '
+	do_pack $object --index-version=2 &&
+	munge $idx $(ofs_table 1) "\377\0\0\0" &&
+	clear_base &&
+	test_must_fail git cat-file blob $object &&
+	test_must_fail git index-pack --verify $pack
+'
+
+test_expect_failure 'bogus offset inside v2 extended table' '
+	# We need two objects here, so we can plausibly require
+	# an extended table (if the first object were larger than 2^31).
+	do_pack "$object $(git rev-parse HEAD)" --index-version=2 &&
+
+	# 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)) &&
+		printf "\200\0\0\0" &&
+		printf "\377\0\0\0\0\0\0\0" &&
+		dd if=$idx bs=1 skip=$(extended_table 2)
+	} >tmp &&
+	mv tmp "$idx" &&
+	clear_base &&
+	test_must_fail git cat-file blob $object &&
+	test_must_fail git index-pack --verify $pack
+'
+
+test_expect_success 'bogus OFS_DELTA in packfile' '
+	# Generate a pack with a delta in it.
+	base=$(test-genrandom foo 3000 | git hash-object --stdin -w) &&
+	delta=$(test-genrandom foo 2000 | git hash-object --stdin -w) &&
+	do_pack "$base $delta" --delta-base-offset &&
+	rm -f .git/objects/??/* &&
+
+	# Double check that we have the delta we expect.
+	echo $base >expect &&
+	echo $delta | git cat-file --batch-check="%(deltabase)" >actual &&
+	test_cmp expect actual &&
+
+	# Now corrupt it. We assume the varint size for the delta is small
+	# enough to fit in the first byte (which it should be, since it
+	# is a pure deletion from the base), and that original ofs_delta
+	# takes 2 bytes (which it should, as it should be ~3000).
+	ofs=$(git show-index <$idx | grep $delta | cut -d" " -f1) &&
+	munge $pack $(($ofs + 1)) "\177\377" &&
+	test_must_fail git cat-file blob $delta >/dev/null
+'
+
+test_done
-- 
2.7.2.695.gf3fde8e

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

[Johannes Sixt]

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

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

[Junio C Hamano]

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.

Thanks.

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

[Jeff King]

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 &&

[PATCH 2/3] nth_packed_object_offset: bounds-check extended offset

[Jeff King]

If a pack .idx file has a corrupted offset for an object, we
may try to access an offset in the .idx or .pack file that
is larger than the file's size.  For the .pack case, we have
use_pack() to protect us, which realizes the access is out
of bounds. But if the corrupted value asks us to look in the
.idx file's secondary 64-bit offset table, we blindly add it
to the mmap'd index data and access arbitrary memory.

We can fix this with a simple bounds-check compared to the
size we found when we opened the .idx file.

Note that there's similar code in index-pack that is
triggered only during "index-pack --verify". To support
both, we pull the bounds-check into a separate function,
which dies when it sees a corrupted file.

It would be nice if we could return an error, so that the
pack code could try to find a good copy of the object
elsewhere. Currently nth_packed_object_offset doesn't have
any way to return an error, but it could probably use "0" as
a sentinel value (since no object can start there). This is
the minimal fix, and we can improve the resilience later on
top.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/index-pack.c          |  1 +
 cache.h                       | 10 ++++++++++
 sha1_file.c                   | 15 +++++++++++++++
 t/t5313-pack-bounds-checks.sh |  2 +-
 4 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 6a01509..ad6a1d7 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1514,6 +1514,7 @@ static void read_v2_anomalous_offsets(struct packed_git *p,
 		if (!(off & 0x80000000))
 			continue;
 		off = off & 0x7fffffff;
+		check_pack_index_ptr(p, &idx2[off * 2]);
 		if (idx2[off * 2])
 			continue;
 		/*
diff --git a/cache.h b/cache.h
index 83b688c..25a7040 100644
--- a/cache.h
+++ b/cache.h
@@ -1370,6 +1370,16 @@ extern void clear_delta_base_cache(void);
 extern struct packed_git *add_packed_git(const char *path, size_t path_len, int local);
 
 /*
+ * Make sure that a pointer access into an mmap'd index file is within bounds,
+ * and can provide at least 8 bytes of data.
+ *
+ * Note that this is only necessary for variable-length segments of the file
+ * (like the 64-bit extended offset table), as we compare the size to the
+ * fixed-length parts when we open the file.
+ */
+extern void check_pack_index_ptr(const struct packed_git *p, const void *ptr);
+
+/*
  * Return the SHA-1 of the nth object within the specified packfile.
  * Open the index if it is not already open.  The return value points
  * at the SHA-1 within the mmapped index.  Return NULL if there is an
diff --git a/sha1_file.c b/sha1_file.c
index aab1872..d2ffd92 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2446,6 +2446,20 @@ const unsigned char *nth_packed_object_sha1(struct packed_git *p,
 	}
 }
 
+void check_pack_index_ptr(const struct packed_git *p, const void *vptr)
+{
+	const unsigned char *ptr = vptr;
+	const unsigned char *start = p->index_data;
+	const unsigned char *end = start + p->index_size;
+	if (ptr < start)
+		die("offset before start of pack index for %s (corrupt index?)",
+		    p->pack_name);
+	/* No need to check for underflow; .idx files must be at least 8 bytes */
+	if (ptr >= end - 8)
+		die("offset beyond end of pack index for %s (truncated index?)",
+		    p->pack_name);
+}
+
 off_t nth_packed_object_offset(const struct packed_git *p, uint32_t n)
 {
 	const unsigned char *index = p->index_data;
@@ -2459,6 +2473,7 @@ off_t nth_packed_object_offset(const struct packed_git *p, uint32_t n)
 		if (!(off & 0x80000000))
 			return off;
 		index += p->num_objects * 4 + (off & 0x7fffffff) * 8;
+		check_pack_index_ptr(p, index);
 		return (((uint64_t)ntohl(*((uint32_t *)(index + 0)))) << 32) |
 				   ntohl(*((uint32_t *)(index + 4)));
 	}
diff --git a/t/t5313-pack-bounds-checks.sh b/t/t5313-pack-bounds-checks.sh
index efc5197..0717746 100755
--- a/t/t5313-pack-bounds-checks.sh
+++ b/t/t5313-pack-bounds-checks.sh
@@ -128,7 +128,7 @@ test_expect_success 'bogus object offset (v2, no msb)' '
 	test_must_fail git index-pack --verify $pack
 '
 
-test_expect_failure 'bogus offset into v2 extended table' '
+test_expect_success 'bogus offset into v2 extended table' '
 	do_pack $object --index-version=2 &&
 	munge $idx $(ofs_table 1) "\377\0\0\0" &&
 	clear_base &&
-- 
2.7.2.695.gf3fde8e

[PATCH 3/3] use_pack: handle signed off_t overflow

[Jeff King]

A v2 pack index file can specify an offset within a packfile
of up to 2^64-1 bytes. On a system with a signed 64-bit
off_t, we can represent only up to 2^63-1. This means that a
corrupted .idx file can end up with a negative offset in the
pack code. Our bounds-checking use_pack function looks for
too-large offsets, but not for ones that have wrapped around
to negative. Let's do so, which fixes an out-of-bounds
access demonstrated in t5313.

Signed-off-by: Jeff King <peff@peff.net>
---
 sha1_file.c                   | 2 ++
 t/t5313-pack-bounds-checks.sh | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/sha1_file.c b/sha1_file.c
index d2ffd92..9d223e6 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1076,6 +1076,8 @@ unsigned char *use_pack(struct packed_git *p,
 		die("packfile %s cannot be accessed", p->pack_name);
 	if (offset > (p->pack_size - 20))
 		die("offset beyond end of packfile (truncated pack?)");
+	if (offset < 0)
+		die("offset before end of packfile (broken .idx?)");
 
 	if (!win || !in_window(win, offset)) {
 		if (win)
diff --git a/t/t5313-pack-bounds-checks.sh b/t/t5313-pack-bounds-checks.sh
index 0717746..a8a587a 100755
--- a/t/t5313-pack-bounds-checks.sh
+++ b/t/t5313-pack-bounds-checks.sh
@@ -136,7 +136,7 @@ test_expect_success 'bogus offset into v2 extended table' '
 	test_must_fail git index-pack --verify $pack
 '
 
-test_expect_failure 'bogus offset inside v2 extended table' '
+test_expect_success 'bogus offset inside v2 extended table' '
 	# We need two objects here, so we can plausibly require
 	# an extended table (if the first object were larger than 2^31).
 	do_pack "$object $(git rev-parse HEAD)" --index-version=2 &&
-- 
2.7.2.695.gf3fde8e

[PATCH 4/3] sha1_file.c: mark strings for translation

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Since jk/pack-idx-corruption-safety is already in 'next', can we add
 this patch on top? Surrounding strings are handled separately [1] by
 another series.

 [1] http://thread.gmane.org/gmane.comp.version-control.git/287661/focus=287678

 sha1_file.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 4a3a032..b8da68b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1042,7 +1042,7 @@ unsigned char *use_pack(struct packed_git *p,
 	if (offset > (p->pack_size - 20))
 		die("offset beyond end of packfile (truncated pack?)");
 	if (offset < 0)
-		die("offset before end of packfile (broken .idx?)");
+		die(_("offset before end of packfile (broken .idx?)"));
 
 	if (!win || !in_window(win, offset)) {
 		if (win)
@@ -2367,11 +2367,11 @@ void check_pack_index_ptr(const struct packed_git *p, const void *vptr)
 	const unsigned char *start = p->index_data;
 	const unsigned char *end = start + p->index_size;
 	if (ptr < start)
-		die("offset before start of pack index for %s (corrupt index?)",
+		die(_("offset before start of pack index for %s (corrupt index?)"),
 		    p->pack_name);
 	/* No need to check for underflow; .idx files must be at least 8 bytes */
 	if (ptr >= end - 8)
-		die("offset beyond end of pack index for %s (truncated index?)",
+		die(_("offset beyond end of pack index for %s (truncated index?)"),
 		    p->pack_name);
 }
 
-- 
2.8.0.rc0.205.g7ec8cf1

Re: [PATCH 4/3] sha1_file.c: mark strings for translation

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  Since jk/pack-idx-corruption-safety is already in 'next', can we add
>  this patch on top? Surrounding strings are handled separately [1] by
>  another series.
>
>  [1] http://thread.gmane.org/gmane.comp.version-control.git/287661/focus=287678

Thanks, I think this makes sense.  Peff--I do not think I missed a
reason we shouldn't take this?

>
>  sha1_file.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/sha1_file.c b/sha1_file.c
> index 4a3a032..b8da68b 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -1042,7 +1042,7 @@ unsigned char *use_pack(struct packed_git *p,
>  	if (offset > (p->pack_size - 20))
>  		die("offset beyond end of packfile (truncated pack?)");
>  	if (offset < 0)
> -		die("offset before end of packfile (broken .idx?)");
> +		die(_("offset before end of packfile (broken .idx?)"));
>  
>  	if (!win || !in_window(win, offset)) {
>  		if (win)
> @@ -2367,11 +2367,11 @@ void check_pack_index_ptr(const struct packed_git *p, const void *vptr)
>  	const unsigned char *start = p->index_data;
>  	const unsigned char *end = start + p->index_size;
>  	if (ptr < start)
> -		die("offset before start of pack index for %s (corrupt index?)",
> +		die(_("offset before start of pack index for %s (corrupt index?)"),
>  		    p->pack_name);
>  	/* No need to check for underflow; .idx files must be at least 8 bytes */
>  	if (ptr >= end - 8)
> -		die("offset beyond end of pack index for %s (truncated index?)",
> +		die(_("offset beyond end of pack index for %s (truncated index?)"),
>  		    p->pack_name);
>  }

Re: [PATCH 4/3] sha1_file.c: mark strings for translation

[Jeff King]

On Sat, Feb 27, 2016 at 09:41:09AM -0800, Junio C Hamano wrote:

> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
> 
> > Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> > ---
> >  Since jk/pack-idx-corruption-safety is already in 'next', can we add
> >  this patch on top? Surrounding strings are handled separately [1] by
> >  another series.
> >
> >  [1] http://thread.gmane.org/gmane.comp.version-control.git/287661/focus=287678
> 
> Thanks, I think this makes sense.  Peff--I do not think I missed a
> reason we shouldn't take this?

No, seems fine to me.

-Peff