[CRASH] git segfaults on invalid binary diff

[CRASH] git segfaults on invalid binary diff

[Rafaël Carré]

[-- Attachment #1: Type: text/plain, Size: 4815 bytes --]

Hi,

I noticed git crashed on a patch generated with:
git show --full-index --pretty=email <commit>

(I had forgotten --binary)

% git --version
git version 1.7.3.1.120.g38a18
% mkdir test
% cd test
% git init
Initialized empty Git repository in /home/fun/test/.git/
% git am ~/bad.diff
Applying: foo bar
Segmentation fault (core dumped)
Patch failed at 0001 foo bar
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
% cat ~/bad.diff
From 2e888fde246ecb0d346ecc02e7df0c82724f02b9 Mon Sep 17 00:00:00 2001
From: John Doe <john.doe@example.org>
Date: Mon, 4 Feb 2008 01:17:33 +0000
Subject: [PATCH] foo
     bar


diff --git a/axvlc.tlb b/axvlc.tlb
new file mode 100644
index 0000000000000000000000000000000000000000..a61182de2f3eb2dc2fc13a08b448b79ae56f1967
Binary files /dev/null and b/axvlc.tlb differ
% gdb git core
GNU gdb (GDB) 7.2-ubuntu
Copyright (C) 2010 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /home/fun/.local/git/bin/git...done.
[New Thread 24102]

warning: Can't read pathname for load map: Erreur d'entrée/sortie.
Reading symbols from /lib/libz.so.1...(no debugging symbols found)...done.
Loaded symbols for /lib/libz.so.1
Reading symbols from /lib/libpthread.so.0...Reading symbols from /usr/lib/debug/lib/libpthread-2.12.1.so...done.
done.
Loaded symbols for /lib/libpthread.so.0
Reading symbols from /lib/libc.so.6...Reading symbols from /usr/lib/debug/lib/libc-2.12.1.so...done.
done.
Loaded symbols for /lib/libc.so.6
Reading symbols from /lib64/ld-linux-x86-64.so.2...Reading symbols from /usr/lib/debug/lib/ld-2.12.1.so...done.
done.
Loaded symbols for /lib64/ld-linux-x86-64.so.2
Core was generated by `git apply --index /home/fun/test/.git/rebase-apply/patch'.
Program terminated with signal 11, Segmentation fault.
#0  0x000000000040c49e in apply_binary_fragment (img=0x7fff47152cd0, 
    patch=0x16ef2a0) at builtin/apply.c:2657
2657		switch (fragment->binary_patch_method) {
(gdb) bt
#0  0x000000000040c49e in apply_binary_fragment (img=0x7fff47152cd0, 
    patch=0x16ef2a0) at builtin/apply.c:2657
#1  0x000000000040c7ef in apply_binary (img=0x7fff47152cd0, patch=0x16ef2a0)
    at builtin/apply.c:2739
#2  0x000000000040c92f in apply_fragments (img=0x7fff47152cd0, patch=0x16ef2a0)
    at builtin/apply.c:2761
#3  0x000000000040cdc5 in apply_data (patch=0x16ef2a0, st=0x7fff47152d60, 
    ce=0x0) at builtin/apply.c:2908
#4  0x000000000040d64a in check_patch (patch=0x16ef2a0) at builtin/apply.c:3098
#5  0x000000000040d6d7 in check_patch_list (patch=0x16ef2a0)
    at builtin/apply.c:3113
#6  0x000000000040ef6d in apply_patch (fd=3, 
    filename=0x7fff471549e7 "/home/fun/test/.git/rebase-apply/patch", 
    options=0) at builtin/apply.c:3683
#7  0x000000000040faee in cmd_apply (argc=1, argv=0x7fff471538a0, prefix_=0x0)
    at builtin/apply.c:3897
#8  0x00000000004048c5 in run_builtin (p=0x766688, argc=3, argv=0x7fff471538a0)
    at git.c:275
#9  0x0000000000404a60 in handle_internal_command (argc=3, argv=0x7fff471538a0)
    at git.c:431
#10 0x0000000000404b5a in run_argv (argcp=0x7fff4715378c, argv=0x7fff47153780)
    at git.c:475
#11 0x0000000000404ce2 in main (argc=3, argv=0x7fff471538a0) at git.c:548
(gdb) frame 0
#0  0x000000000040c49e in apply_binary_fragment (img=0x7fff47152cd0, 
    patch=0x16ef2a0) at builtin/apply.c:2657
2657		switch (fragment->binary_patch_method) {
(gdb) print *patch
$1 = {new_name = 0x16ec0e0 "axvlc.tlb", old_name = 0x0, 
  def_name = 0x16ec0e0 "axvlc.tlb", old_mode = 0, new_mode = 33188, 
  is_new = 1, is_delete = 0, rejected = 1, ws_rule = 19, deflate_origlen = 0, 
  lines_added = 0, lines_deleted = 0, score = 0, is_toplevel_relative = 1, 
  inaccurate_eof = 0, is_binary = 1, is_copy = 0, is_rename = 0, recount = 0, 
  fragments = 0x0, result = 0x0, resultsize = 0, 
  old_sha1_prefix = '0' <se r\377\377\377\377\377������\000\000\000\377\377\377\377\377������������������������������������������������������������������, 
  new_sha1_prefix = "a61182de2f3eb2dc2fc13a08b448b79ae56f1967", next = 0x0}
(gdb) 


-- 
✍ Rafaël Carré ☺

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [CRASH] git segfaults on invalid binary diff

[Jeff King]

On Mon, Oct 18, 2010 at 06:06:32PM +0200, Rafaël Carré wrote:

> I noticed git crashed on a patch generated with:
> git show --full-index --pretty=email <commit>
> 
> (I had forgotten --binary)
> 
> % git --version
> git version 1.7.3.1.120.g38a18
> % mkdir test
> % cd test
> % git init
> Initialized empty Git repository in /home/fun/test/.git/
> % git am ~/bad.diff
> Applying: foo bar
> Segmentation fault (core dumped)
> Patch failed at 0001 foo bar
> When you have resolved this problem run "git am --resolved".
> If you would prefer to skip this patch, instead run "git am --skip".
> To restore the original branch and stop patching run "git am --abort".
> % cat ~/bad.diff

Hmm, the key seems to be applying it in a new repo without the actual
objects. Git will fall back to trying to apply based on the index line,
and that is presumably where the bug is. I'll try to work up a patch.

-Peff

[PATCH] apply: don't segfault on binary files with missing data

[Jeff King]

Usually when applying a binary diff generated without
--binary, it will be rejected early, as we don't even have
the full sha1 of the pre- and post-images.

However, if the diff is generated with --full-index (but not
--binary), then we will actually try to apply it. If we have
the postimage blob, then we can take a shortcut and never
even look at the binary diff at all (e.g., this can happen
when rebasing changes within a repository).

If we don't have the postimage blob, though, we try to look
at the actual fragments, of which there are none, and get a
segfault. This patch checks explicitly for that case and
complains to the user instead of segfaulting. We need to
keep the check at a low level so that the "shortcut" case
above continues to work.

We also add a test that demonstrates the segfault. While
we're at it, let's also explicitly test the shortcut case.

Reported-by: Rafaël Carré <rafael.carre@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/apply.c         |    6 ++++++
 t/t4103-apply-binary.sh |   27 ++++++++++++++++++++++++++-
 2 files changed, 32 insertions(+), 1 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 23c18c5..f051e66 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -2645,6 +2645,12 @@ static int apply_binary_fragment(struct image *img, struct patch *patch)
 	unsigned long len;
 	void *dst;
 
+	if (!fragment)
+		return error("missing binary patch data for '%s'",
+			     patch->new_name ?
+			     patch->new_name :
+			     patch->old_name);
+
 	/* Binary patch is irreversible without the optional second hunk */
 	if (apply_in_reverse) {
 		if (!fragment->next)
diff --git a/t/t4103-apply-binary.sh b/t/t4103-apply-binary.sh
index 9692f16..08ad6d8 100755
--- a/t/t4103-apply-binary.sh
+++ b/t/t4103-apply-binary.sh
@@ -37,7 +37,16 @@ test_expect_success 'setup' "
 	git diff-tree -p -C master binary >C.diff &&
 
 	git diff-tree -p --binary master binary >BF.diff &&
-	git diff-tree -p --binary -C master binary >CF.diff
+	git diff-tree -p --binary -C master binary >CF.diff &&
+
+	git diff-tree -p --full-index master binary >B-index.diff &&
+	git diff-tree -p -C --full-index master binary >C-index.diff &&
+
+	git init other-repo &&
+	(cd other-repo &&
+	 git fetch .. master &&
+	 git reset --hard FETCH_HEAD
+	)
 "
 
 test_expect_success 'stat binary diff -- should not fail.' \
@@ -100,6 +109,22 @@ test_expect_success 'apply binary diff (copy) -- should fail.' \
 	'do_reset &&
 	 test_must_fail git apply --index C.diff'
 
+test_expect_success 'apply binary diff with full-index' '
+	do_reset &&
+	git apply B-index.diff
+'
+
+test_expect_success 'apply binary diff with full-index (copy)' '
+	do_reset &&
+	git apply C-index.diff
+'
+
+test_expect_success 'apply full-index binary diff in new repo' '
+	(cd other-repo &&
+	 do_reset &&
+	 test_must_fail git apply ../B-index.diff)
+'
+
 test_expect_success 'apply binary diff without replacement.' \
 	'do_reset &&
 	 git apply BF.diff'
-- 
1.7.3.1.227.ge6319.dirty