search

rockcarry / ffjpeg

a simple jpeg codec.
GNU General Public License v3.0
106 stars 46 forks source link

Heap-buffer-overflows in jfif_decode() at jfif.c:552:31 and 552:38 #43

Closed cemonatk closed 2 years ago

cemonatk commented 3 years ago

Describe Two Heap-buffer-overflows were discovered in ffjpeg. The issues are being triggered in function jfif_decode at jfif.c:552:31 and 552:38.

Found by Cem Onat Karagun of Diesec

System info OS version : Ubuntu 21.04 ffjpeg Version : master 0fa4cf8a86

Reproduce

Compile ffjpeg with address sanitizer. 

CCFLAGS = -Wall -g -fsanitize=address

POC Files: decode_poc1.zip decode_poc2.zip

Run POCs with the commands below.

$ ffjpeg -d decode_poc1

$ ffjpeg -d decode_poc2

Asan output-1:

=================================================================
==3469985==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000000010 at pc 0x00000030d50c bp 0x7ffc93e360f0 sp 0x7ffc93e360e8
READ of size 4 at 0x602000000010 thread T0
    #0 0x30d50b in jfif_decode /src/src/jfif.c:552:31
    #1 0x3035f0 in main /src/src/ffjpeg.c:24:9
    #2 0x7f3772bef564 in __libc_start_main csu/../csu/libc-start.c:332:16
    #3 0x2515fd in _start (/REDACTED/ffjpeg/src/ffjpeg+0x2515fd)

0x602000000011 is located 0 bytes to the right of 1-byte region [0x602000000010,0x602000000011)
allocated by thread T0 here:
    #0 0x2cc85d in malloc (/REDACTED/ffjpeg/src/ffjpeg+0x2cc85d)
    #1 0x309025 in jfif_decode /src/src/jfif.c:443:21
    #2 0x3035f0 in main /src/src/ffjpeg.c:24:9
    #3 0x7f3772bef564 in __libc_start_main csu/../csu/libc-start.c:332:16

SUMMARY: AddressSanitizer: heap-buffer-overflow /src/src/jfif.c:552:31 in jfif_decode
Shadow bytes around the buggy address:
  0x0c047fff7fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c047fff8000: fa fa[01]fa fa fa 01 fa fa fa 01 fa fa fa fa fa
  0x0c047fff8010: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8020: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8050: 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
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:

Asan output-2:

=================================================================
==3487109==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000000010 at pc 0x00000030d52a bp 0x7ffe17b50930 sp 0x7ffe17b50928
READ of size 4 at 0x602000000010 thread T0
    #0 0x30d529 in jfif_decode /src/src/jfif.c:552:38
    #1 0x3035f0 in main /src/src/ffjpeg.c:24:9
    #2 0x7fec13038564 in __libc_start_main csu/../csu/libc-start.c:332:16
    #3 0x2515fd in _start (/REDACTED/ffjpeg/src/ffjpeg+0x2515fd)

0x602000000011 is located 0 bytes to the right of 1-byte region [0x602000000010,0x602000000011)
allocated by thread T0 here:
    #0 0x2cc85d in malloc (/REDACTED/ffjpeg/src/ffjpeg+0x2cc85d)
    #1 0x309068 in jfif_decode /src/src/jfif.c:444:21
    #2 0x3035f0 in main /src/src/ffjpeg.c:24:9
    #3 0x7fec13038564 in __libc_start_main csu/../csu/libc-start.c:332:16

SUMMARY: AddressSanitizer: heap-buffer-overflow /src/src/jfif.c:552:38 in jfif_decode
Shadow bytes around the buggy address:
  0x0c047fff7fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c047fff8000: fa fa[01]fa fa fa 01 fa fa fa fa fa fa fa fa fa
  0x0c047fff8010: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8020: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8050: 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
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==3487109==ABORTING
Marsman1996 commented 2 years ago

This issue is the same as #28 (assigned with CVE-2020-23852) and #24 (assigned CVE-2020-13439).

The overflow is caused by the accessing to the memory buffer without checking the boundary. https://github.com/rockcarry/ffjpeg/blob/0fa4cf8a86d7f23a3e8336343c1895aa634fdc76/src/jfif.c#L550-L552

ffjpeg access yuv_datbuf[1] with offset uy * yuv_stride[1] + ux without checking whether it is over the buffer size https://github.com/rockcarry/ffjpeg/blob/0fa4cf8a86d7f23a3e8336343c1895aa634fdc76/src/jfif.c#L443-L444

An ugly solution is to add check before the access, such as

if (usrc >= yuv_datbuf[1] + yuv_stride[1] * yuv_height[1] * sizeof(int)) {
    printf("usrc overflow\n");
    return -1;
}
if (vsrc >= yuv_datbuf[2] + yuv_stride[2] * yuv_height[2] * sizeof(int)) {
    printf("vsrc overflow\n");
    return -1;
}