AddressSanitizer: heap-buffer-overflow in jfif_load() at jfif.c:187

Marsman1996 commented 5 years ago

Test Environment

Ubuntu 14.04, 64bit, ffjpeg(master 627c8a9)

How to trigger

compile ffjpeg with cmake file from https://github.com/rockcarry/ffjpeg/issues/6
$ ./ffjpeg -d $POC

POC file

https://github.com/Marsman1996/pocs/blob/master/ffjpeg/poc22-jfif_load-heapoverflow

Details

Asan report

=================================================================
==17308== ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60340000fef8 at pc 0x402fec bp 0x7ffc051db980 sp 0x7ffc051db978
WRITE of size 4 at 0x60340000fef8 thread T0
    #0 0x402feb in jfif_load /home/aota10/MARS_fuzzcompare/test/ffjpeg/code/jfif.c:187
    #1 0x401a59 in main /home/aota10/MARS_fuzzcompare/test/ffjpeg/code/ffjpeg.c:24
    #2 0x7f5291e9bf44 (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)
    #3 0x401858 in _start (/home/aota10/MARS_fuzzcompare/test/ffjpeg/bin_asan/bin/ffjpeg+0x401858)
0x60340000fef8 is located 0 bytes to the right of 504-byte region [0x60340000fd00,0x60340000fef8)
allocated by thread T0 here:
    #0 0x7f52922584e5 (/usr/lib/x86_64-linux-gnu/libasan.so.0+0x154e5)
    #1 0x40293b in jfif_load /home/aota10/MARS_fuzzcompare/test/ffjpeg/code/jfif.c:154
    #2 0x401a59 in main /home/aota10/MARS_fuzzcompare/test/ffjpeg/code/ffjpeg.c:24
    #3 0x7f5291e9bf44 (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)
SUMMARY: AddressSanitizer: heap-buffer-overflow /home/aota10/MARS_fuzzcompare/test/ffjpeg/code/jfif.c:187 jfif_load
Shadow bytes around the buggy address:
  0x0c06ffff9f80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c06ffff9f90: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c06ffff9fa0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c06ffff9fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c06ffff9fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c06ffff9fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00[fa]
  0x0c06ffff9fe0:fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c06ffff9ff0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c06ffffa000: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c06ffffa010: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c06ffffa020: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 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 righ 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
  ASan internal:         fe
==17308== ABORTING

GDB report

Starting program: /home/aota10/MARS_fuzzcompare/test/ffjpeg/build_normal/ffjpeg -d poc22-jfif_load-heapoverflow 
file eof !
*** Error in `/home/aota10/MARS_fuzzcompare/test/ffjpeg/build_normal/ffjpeg': munmap_chunk(): invalid pointer: 0x000000000060a210 ***

Program received signal SIGABRT, Aborted.
0x00007ffff7a47c37 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
56      ../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  0x00007ffff7a47c37 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1  0x00007ffff7a4b028 in __GI_abort () at abort.c:89
#2  0x00007ffff7a842a4 in __libc_message (do_abort=do_abort@entry=1, 
    fmt=fmt@entry=0x7ffff7b96350 "*** Error in `%s': %s: 0x%s ***\n") at ../sysdeps/posix/libc_fatal.c:175
#3  0x00007ffff7a8f007 in malloc_printerr (action=<optimized out>, 
    str=0x7ffff7b966d0 "munmap_chunk(): invalid pointer", ptr=<optimized out>) at malloc.c:4998
#4  0x0000000000402416 in jfif_load (file=0x7fffffffe58c "poc22-jfif_load-heapoverflow")
    at /home/aota10/MARS_fuzzcompare/test/ffjpeg/code/jfif.c:257
#5  0x000000000040165b in main (argc=3, argv=0x7fffffffe298)
    at /home/aota10/MARS_fuzzcompare/test/ffjpeg/code/ffjpeg.c:24
(gdb)

rockcarry commented 5 years ago

i make a new commit

commit b3039ae8022da67078c130bd19bc3008a037adb3

which fix issue #10 #11 and #12

can you test again ?

Marsman1996 commented 5 years ago

Hi, I tested it with commit b3039ae

but there still exists some buffer overflows in jfif.c:195; jfif.c:216, jfif.c:228 and jfif.c:231

here are the POCs and asan log asan.log ffjpeg_poc.zip

rockcarry commented 5 years ago

pls test commit 6bc54edd36fa7b984aa134725f4748d46e372c7a

Marsman1996 commented 5 years ago

Hi,

I tested it with commit 6bc54ed

and AddressSanitizer still reports heap-buffer-overflow in jfif.c:216 and jfif.c:231, which is caused by id:000026 & id:000029 in the ffjpeg_poc.zip above.

to use AddressSanitizer you can compile program with cmd

$ CC="gcc -fsanitize=address -g" CXX="g++ -fsanitize=address -g"  cmake .
$ make

Cheers

rockcarry commented 5 years ago

are the test case 16 & 25 pass your testing ? and the case 26 & 29 still failed ?

I don't have the test enviroment, can you tell me how to install the AddressSanitizer?

Marsman1996 commented 5 years ago

Hi,

are the test case 16 & 25 pass your testing ? and the case 26 & 29 still failed ?

Yes, 26 & 29 still fail while 16 & 25 pass.

I don't have the test enviroment, can you tell me how to install the AddressSanitizer?

AddressSanitizer is part of gcc since gcc-4.8, ~~not sure whether it can be installed separately.~~ Here (ASANwiki) is the guide to build the ASAN from source, though I think use ASAN in gcc cost less effort

Cheers

rockcarry commented 5 years ago

please test commit d003dd61abfe7ab68fd89bbdb5608882ac128e61

Marsman1996 commented 5 years ago

Hi,

I tested commit 2ad299a and ASAN reports heap buffer overflow in https://github.com/rockcarry/ffjpeg/blob/2ad299a390c63895333f8ea51ff54f8765678494/jfif.c#L228

Maybe it should be changed to

for (i=1; i<1+16 && dht+i<end; i++) len += dht[i];

This would fix the overflow problem but I'm not sure if this logic is right.

Best wishes

rockcarry commented 5 years ago

pls test commit 58b6f94b82c4d81f5bd237d919c907cf62853680 i think it‘s ok now.

Marsman1996 commented 5 years ago

Hi,

I tested commit 58b6f94. Since no more ASAN reports, I think this series of heap-overflow problems are fixed now.

Cheers!

rockcarry / ffjpeg