wernsey / bitmap

A C module for manipulating bitmap/raster graphics
MIT No Attribution
69 stars 12 forks source link

Heap Buffer Overflow while loading bmp files and some other minor bugs #9

Closed X3eRo0 closed 3 years ago

X3eRo0 commented 3 years ago

Heap Buffer Overflow -- bmp.c:813

-----------------------------------------------------------------------
813: if(rd.fread(data, 1, dib.bmp_bytesz, rd.data) != dib.bmp_bytesz) {
-----------------------------------------------------------------------

the bmp_bytesz attribute is controlled by the attacker and the
on line 813, a memory read takes place from the input file and
the data allocated by malloc. 

The data is allocated for width and height of image.

-----------------------------------------------------
801: data = CAST(unsigned char *)(malloc(rs * b->h));
-----------------------------------------------------

Potentially Exploitable

--

Fix

    if(dib.bmp_bytesz == 0) {
        // allocate data according to row and height

        data = CAST(unsigned char *)(malloc(rs * b->h));
        if(!data) {
            SET_ERROR("out of memory");
            goto error;
        }

        if(rd.fread(data, 1, rs * b->h, rd.data) != rs * b->h) {
            SET_ERROR("fread on data");
            goto error;
        }

    } else {

        // allocate data according to image length

        data = CAST(unsigned char *)(malloc(dib.bmp_bytesz));
        if(!data) {
            SET_ERROR("out of memory");
            goto error;
        }

        if(rd.fread(data, 1, dib.bmp_bytesz, rd.data) != dib.bmp_bytesz) {
            SET_ERROR("fread on data");
            goto error;
        }
    }

Integer Overflow Leading to Out of Bounds Memory access -- bmp.c:6842, bmp.c:874, bmp.c:859, bmp.c:844, bmp.c:834

the dimentions of the image can be set to very large values resulting in integer overflow when allocating b->data.

sometimes this value is so large that that b->data often returns
NULL

-------------------------------------------------------------------
279: b->data = CAST(unsigned char *)(malloc(b->w * b->h * BM_BPP));
-------------------------------------------------------------------

which can later cause an out of bounds memory access when
setting the RGBA color.

--------------------------------------------------
798: rs = ((dib.width * dib.bitspp / 8) + 3) & ~3;
--------------------------------------------------

rs is checked to be a multiple of 4, if not the program aborts.

-----------------------------------------------------------------
873: int p = y * rs + i * 3;
874: BM_SET_RGBA(b, i, j, data[p+2], data[p+1], data[p+0], 0xFF);
-----------------------------------------------------------------

on line 6842, the result of bmp->w * bmp->h * BM_BPP may cross the 32 bit range and overflow to a smaller number which then may cause an out of bounds write in BM_SET

-----------------------------------
6842: BM_SET(b, x + i, y + j, col);
-----------------------------------

pwndbg> p/x b->h
$13 = 0xa64d
pwndbg> p/x b->w
$14 = 0x8000
pwndbg> p/x (0xa64d * 0x8000) * 4                                # malloc(0x4c9a0000)
$20 = 0x4c9a0000                         # it should be 0x14c9a0000
pwndbg> p/x (0x5326 * b->w + 0x4001) * 4 # BM_SET accessing memory
$16 = 0xa64d0004                         # 0xa64d004 > 0x4c9a0000

The same issue also exists on line no. 859, 846, 834, 6842

-----------------------------------------------------------
858: uint32_t* pixel = (uint32_t*)(data + p); // p out of range
859: uint32_t r_unc = (*pixel & rgbmask[0]) >> rgbshift[0];
-----------------------------------------------------------

-----------------------------------------------------------
844: int byt = y * rs + (i >> 3); // byt is out of range
845: int bit = 7 - (i % 8);
846: uint8_t p = (data[byt] & (1 << bit)) >> bit;
-----------------------------------------------------------

-----------------------------------------------------------
834: int byt = y * rs + (i >> 1); // byt is out of range
835: uint8_t p = ( (i & 0x01) ? data[byt] : (data[byt] >> 4) ) & 0x0F;
-----------------------------------------------------------

Fix

I think width and height must be limited to a value such that calculating ROW_SIZE never overflows the 32 bit range.

If i made a mistake somewhere please dont mind it. i have a bunch of corpses i got during fuzzing. Crashing BMPs

wernsey commented 3 years ago

I'm not ignoring this issue, I just haven't gotten around to do anything about it yet.

wernsey commented 3 years ago

I've committed a fix in 905834c