[patch] Out of bounds read in git apply on malformed input in gitdiff_verify_name()

[patch] Out of bounds read in git apply on malformed input in gitdiff_verify_name()

[Hanno Böck]

[-- Attachment #1.1: Type: text/plain, Size: 4655 bytes --]

Hi,

The attached sample file will cause an out of bounds heap read in the
function gitdiff_verify_name() when passed to git apply.

To reproduce:
* Build git with address sanitizer (-fsanitize=address in cflags)
* git apply --check [path_to_file]

I have pasted the full address sanitizer error trace below.

The code where this happens is here (builtin/apply.c):
                if (!another || memcmp(another, orig_name, len + 1))

The problem seems to be that there is no guarantee that another and
orig_name have the same length, len is set to strlen(orig_name) a few
lines earlier. Thus if orig_name is more than 1 byte shorter than
another it causes an invalid memory read.

Changing memcmp to strncmp should fix this, patch attached.

Found with the help of american fuzzy lop and address sanitizer.


==7717==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000ed30 at pc 0x41d3ab bp 0x7ffd00ad61d0 sp 0x7ffd00ad61c0
READ of size 3 at 0x60200000ed30 thread T0
    #0 0x41d3aa in gitdiff_verify_name builtin/apply.c:940
    #1 0x41d687 in gitdiff_oldname builtin/apply.c:956
    #2 0x41f387 in parse_git_header builtin/apply.c:1309
    #3 0x42014b in find_header builtin/apply.c:1475
    #4 0x422bc5 in parse_chunk builtin/apply.c:1988
    #5 0x432c24 in apply_patch builtin/apply.c:4373
    #6 0x433df8 in cmd_apply builtin/apply.c:4626
    #7 0x407dcd in run_builtin /f/git/plain/git-2.9.0/git.c:352
    #8 0x40818c in handle_builtin /f/git/plain/git-2.9.0/git.c:539
    #9 0x4084db in run_argv /f/git/plain/git-2.9.0/git.c:593
    #10 0x408934 in main /f/git/plain/git-2.9.0/git.c:698
    #11 0x7fddbf96c78f in __libc_start_main (/lib64/libc.so.6+0x2078f)
    #12 0x405308 in _start (/f/git/plain/git-2.9.0/git+0x405308)

0x60200000ed32 is located 0 bytes to the right of 2-byte region [0x60200000ed30,0x60200000ed32)
allocated by thread T0 here:
    #0 0x7fddc07b9707 in malloc (/usr/lib/gcc/x86_64-pc-linux-gnu/4.9.3/libasan.so.1+0x57707)
    #1 0x790768 in do_xmalloc /f/git/plain/git-2.9.0/wrapper.c:59
    #2 0x79089e in do_xmallocz /f/git/plain/git-2.9.0/wrapper.c:99
    #3 0x79090f in xmallocz /f/git/plain/git-2.9.0/wrapper.c:107
    #4 0x79094c in xmemdupz /f/git/plain/git-2.9.0/wrapper.c:123
    #5 0x41c1f7 in find_name_common builtin/apply.c:704
    #6 0x41c29b in find_name builtin/apply.c:715
    #7 0x41d31b in gitdiff_verify_name builtin/apply.c:939
    #8 0x41d687 in gitdiff_oldname builtin/apply.c:956
    #9 0x41f387 in parse_git_header builtin/apply.c:1309
    #10 0x42014b in find_header builtin/apply.c:1475
    #11 0x422bc5 in parse_chunk builtin/apply.c:1988
    #12 0x432c24 in apply_patch builtin/apply.c:4373
    #13 0x433df8 in cmd_apply builtin/apply.c:4626
    #14 0x407dcd in run_builtin /f/git/plain/git-2.9.0/git.c:352
    #15 0x40818c in handle_builtin /f/git/plain/git-2.9.0/git.c:539
    #16 0x4084db in run_argv /f/git/plain/git-2.9.0/git.c:593
    #17 0x408934 in main /f/git/plain/git-2.9.0/git.c:698
    #18 0x7fddbf96c78f in __libc_start_main (/lib64/libc.so.6+0x2078f)

SUMMARY: AddressSanitizer: heap-buffer-overflow builtin/apply.c:940 gitdiff_verify_name
Shadow bytes around the buggy address:
  0x0c047fff9d50: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9d60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9d70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9d80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9d90: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c047fff9da0: fa fa fa fa fa fa[02]fa fa fa 03 fa fa fa fd fd
  0x0c047fff9db0: fa fa 03 fa fa fa 00 02 fa fa fd fd fa fa 00 00
  0x0c047fff9dc0: fa fa 04 fa fa fa 00 03 fa fa fd fd fa fa 00 00
  0x0c047fff9dd0: fa fa 01 fa fa fa 00 06 fa fa 05 fa fa fa 00 04
  0x0c047fff9de0: fa fa 04 fa fa fa 06 fa fa fa 00 04 fa fa 00 04
  0x0c047fff9df0: fa fa 00 04 fa fa 00 04 fa fa fd fa fa fa 02 fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Contiguous container OOB:fc
  ASan internal:           fe
==7717==ABORTING

-- 
Hanno Böck
https://hboeck.de/

mail/jabber: hanno@hboeck.de
GPG: BBB51E42

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: git-fix-oob-memcmp-strncmp.diff --]
[-- Type: text/x-patch, Size: 703 bytes --]

diff -Naur git-2.9.0/builtin/apply.c git-2.9.0-1/builtin/apply.c
--- git-2.9.0/builtin/apply.c	2016-06-13 21:07:49.000000000 +0200
+++ git-2.9.0-1/builtin/apply.c	2016-06-14 11:38:19.940588382 +0200
@@ -937,7 +937,7 @@
 			die(_("git apply: bad git-diff - expected /dev/null, got %s on line %d"),
 			    orig_name, linenr);
 		another = find_name(line, NULL, p_value, TERM_TAB);
-		if (!another || memcmp(another, orig_name, len + 1))
+		if (!another || strncmp(another, orig_name, len + 1))
 			die((side == DIFF_NEW_NAME) ?
 			    _("git apply: bad git-diff - inconsistent new filename on line %d") :
 			    _("git apply: bad git-diff - inconsistent old filename on line %d"), linenr);

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.3: git-out-of-bounds-gitdiff_verify_name.patch --]
[-- Type: text/x-patch, Size: 30 bytes --]

diff --git 
--- /00
--- /0

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]