rockcarry / ffjpeg

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

Integer overflow resulting in heap overflow in function bmp_load() in bmp.c #38

Open chibataiki opened 3 years ago

chibataiki commented 3 years ago

Hi, an exploitable heap overflow vulnerability exists in function bmp_load() in bmp.c.

version 4ab404e (latest one)

env ubuntu 20.04 x86_64 gcc version 9.3.0

reproduce: make ./ffjpeg -e poc zipped poc

detail

in the function bmp_load() ,the struction of BMP is

typedef struct
{
    int   width;    
    int   height;   
    int   stride;  
    void *pdata;   
} BMP;

The stride type is int, if width is big enough, the stride value will tigger integer overflow.

Also: pb->stride * pb->height will also tigger integer overflow , then pdata will minus the wrong length at line 50.

At line 51 , fread read data from file and copied to the allocated buffer. The pb->stride is bigger than pb->stride * pb->height and will lead to heap corruption.

45          pb->stride = ALIGN(pb->width * 3, 4);
46          pb->pdata  = malloc((size_t)pb->stride * pb->height);
47          if (pb->pdata) {
48              pdata  = (BYTE*)pb->pdata + pb->stride * pb->height;
49              for (i=0; i<pb->height; i++) {
50                  pdata -= pb->stride;
51                  fread(pdata, pb->stride, 1, fp);
──────── threads ────
[#0] Id 1, Name: "ffjpeg", stopped 0x7ffff7f54774 in __memmove_avx_unaligned_erms (), reason: SIGSEGV
────── trace ────
[#0] 0x7ffff7f54774 → __memmove_avx_unaligned_erms()
[#1] 0x7ffff7e584de → __GI__IO_file_xsgetn(fp=0x5555555602a0, data=<optimized out>, n=0x2c000000)
[#2] 0x7ffff7e4c063 → __GI__IO_fread(buf=0x7ffecbdc5010, size=0x2c000000, count=0x1, fp=0x5555555602a0)
[#3] 0x555555555f4a → bmp_load(pb=0x7fffffffe270, file=0x7fffffffe644 "poc")
[#4] 0x5555555574ea → main()
────────
gef➤  frame 3
#3  0x0000555555555f4a in bmp_load (pb=0x7fffffffe270, file=0x7fffffffe644 "poc") at bmp.c:51
51                  fread(pdata, pb->stride, 1, fp);

gef➤  p pb->width
$19 = 0x64000000
gef➤  p pb->height
$20 = 0x6
gef➤  p pb->stride
$52 = 0x2c000000  // integer overflow  

gef➤  p pb->pdata
$54 = (void *) 0x7ffeefdc5010
gef➤  p pb->stride * pb->height
$55 = 0x8000000  // integer overflow
gef➤  p pdata    // write access violation
$56 = (BYTE *) 0x7ffecbdc5010 <error: Cannot access memory at address 0x7ffecbdc5010>
chibataiki commented 3 years ago

I submitted to huntr.dev for a quick cve assignment.

rockcarry commented 3 years ago

fixed, pls check last code.

chibataiki commented 3 years ago

Seem not fix the root cause of the issue : the type of BMP struct stride is int and result integer overflow, fix this maybe need more work. This testcase issue fixed so this patch LGTM, thanks for your work!