Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ebb726f9 authored by Ray Essick's avatar Ray Essick
Browse files

defensive parsing of mp3 album art information 

several points in stagefrights mp3 album art code
used strlen() to parse user-supplied strings that may be
unterminated, resulting in reading beyond the end of a buffer.

This changes the code to use strnlen() for 8-bit encodings and
strengthens the parsing of 16-bit encodings similarly. It also
reworks how we watch for the end-of-buffer to avoid all over-reads.

Bug: 32377688
Test: crafted mp3's w/ good/bad cover art. See what showed in play music
Change-Id: I802a71c6e5968aefde21eb1612b720ff6d579988
parent bf2e61f5

media/libstagefright/id3/ID3.cpp

+39 −17
Original line number Diff line number Diff line
@@ -839,20 +839,21 @@ void ID3::Iterator::findFrame() { 
    }

}



static size_t StringSize(const uint8_t *start, uint8_t encoding) {

// return includes terminator;  if unterminated, returns > limit

static size_t StringSize(const uint8_t *start, size_t limit, uint8_t encoding) {



    if (encoding == 0x00 || encoding == 0x03) {

        // ISO 8859-1 or UTF-8

        return strlen((const char *)start) + 1;

        return strnlen((const char *)start, limit) + 1;

    }



    // UCS-2

    size_t n = 0;

    while (start[n] != '\0' || start[n + 1] != '\0') {

    while ((n+1 < limit) && (start[n] != '\0' || start[n + 1] != '\0')) {

        n += 2;

    }



    // Add size of null termination.

    return n + 2;

    n += 2;

    return n;

}



const void *
@@ -873,11 +874,19 @@ ID3::getAlbumArt(size_t *length, String8 *mime) const { 


        if (mVersion == ID3_V2_3 || mVersion == ID3_V2_4) {

            uint8_t encoding = data[0];

            mime->setTo((const char *)&data[1]);

            size_t mimeLen = strlen((const char *)&data[1]) + 1;

            size_t consumed = 1;



            // *always* in an 8-bit encoding

            size_t mimeLen = StringSize(&data[consumed], size - consumed, 0x00);

            if (mimeLen > size - consumed) {

                ALOGW("bogus album art size: mime");

                return NULL;

            }

            mime->setTo((const char *)&data[consumed]);

            consumed += mimeLen;



#if 0

            uint8_t picType = data[1 + mimeLen];

            uint8_t picType = data[consumed];

            if (picType != 0x03) {

                // Front Cover Art

                it.next();
@@ -885,20 +894,30 @@ ID3::getAlbumArt(size_t *length, String8 *mime) const { 
            }

#endif



            size_t descLen = StringSize(&data[2 + mimeLen], encoding);

            consumed++;

            if (consumed >= size) {

                ALOGW("bogus album art size: pic type");

                return NULL;

            }



            if (size < 2 ||

                    size - 2 < mimeLen ||

                    size - 2 - mimeLen < descLen) {

                ALOGW("bogus album art sizes");

            size_t descLen = StringSize(&data[consumed], size - consumed, encoding);

            consumed += descLen;



            if (consumed >= size) {

                ALOGW("bogus album art size: description");

                return NULL;

            }

            *length = size - 2 - mimeLen - descLen;



            return &data[2 + mimeLen + descLen];

            *length = size - consumed;



            return &data[consumed];

        } else {

            uint8_t encoding = data[0];



            if (size <= 5) {

                return NULL;

            }



            if (!memcmp(&data[1], "PNG", 3)) {

                mime->setTo("image/png");

            } else if (!memcmp(&data[1], "JPG", 3)) {
@@ -918,7 +937,10 @@ ID3::getAlbumArt(size_t *length, String8 *mime) const { 
            }

#endif



            size_t descLen = StringSize(&data[5], encoding);

            size_t descLen = StringSize(&data[5], size - 5, encoding);

            if (descLen > size - 5) {

                return NULL;

            }



            *length = size - 5 - descLen;