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

Commit 5759c46d authored by Ray Essick's avatar Ray Essick
Browse files

DO NOT MERGE: 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: I479d51e88d3180461cb6ea5540974671cfd84201
(cherry picked from commit 52d02b97)
parent b6cf6d6a

media/libstagefright/id3/ID3.cpp

+39 −17
Original line number Diff line number Diff line
@@ -828,20 +828,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 *
@@ -859,11 +860,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;



            uint8_t picType = data[1 + mimeLen];

#if 0

            uint8_t picType = data[1 + mimeLen];

            if (picType != 0x03) {

                // Front Cover Art

                it.next();
@@ -871,20 +880,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;

            }



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

            consumed += descLen;



            if (size < 2 ||

                    size - 2 < mimeLen ||

                    size - 2 - mimeLen < descLen) {

                ALOGW("bogus album art sizes");

            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)) {
@@ -904,7 +923,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;