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

Commit 12d4a6a2 authored by Wonsik Kim's avatar Wonsik Kim
Browse files

DO NOT MERGE Avoid size_t overflow in base64 decoding once again 

Switch to foundation base64 function in OggExtractor and fix the
issue there.

Bug: 23707088
Change-Id: If8ba3347c213fe7a36668c943ed264f2871ad468
parent dffe5cdb

media/libstagefright/OggExtractor.cpp

+12 −90
Original line number Diff line number Diff line
@@ -23,6 +23,7 @@ 
#include <cutils/properties.h>

#include <media/stagefright/foundation/ABuffer.h>

#include <media/stagefright/foundation/ADebug.h>

#include <media/stagefright/foundation/base64.h>

#include <media/stagefright/DataSource.h>

#include <media/stagefright/MediaBuffer.h>

#include <media/stagefright/MediaBufferGroup.h>
@@ -1202,93 +1203,18 @@ void parseVorbisComment( 


}



// The returned buffer should be free()d.

static uint8_t *DecodeBase64(const char *s, size_t size, size_t *outSize) {

    *outSize = 0;



    if ((size % 4) != 0) {

        return NULL;

    }



    size_t n = size;

    size_t padding = 0;

    if (n >= 1 && s[n - 1] == '=') {

        padding = 1;



        if (n >= 2 && s[n - 2] == '=') {

            padding = 2;

        }

    }



    // We divide first to avoid overflow. It's OK to do this because we

    // already made sure that size % 4 == 0.

    size_t outLen = (size / 4) * 3 - padding;



    void *buffer = malloc(outLen);

    if (buffer == NULL) {

        return NULL;

    }



    uint8_t *out = (uint8_t *)buffer;

    size_t j = 0;

    uint32_t accum = 0;

    for (size_t i = 0; i < n; ++i) {

        char c = s[i];

        unsigned value;

        if (c >= 'A' && c <= 'Z') {

            value = c - 'A';

        } else if (c >= 'a' && c <= 'z') {

            value = 26 + c - 'a';

        } else if (c >= '0' && c <= '9') {

            value = 52 + c - '0';

        } else if (c == '+') {

            value = 62;

        } else if (c == '/') {

            value = 63;

        } else if (c != '=') {

            break;

        } else {

            if (i < n - padding) {

                break;

            }



            value = 0;

        }



        accum = (accum << 6) | value;



        if (((i + 1) % 4) == 0) {

            out[j++] = (accum >> 16);



            if (j < outLen) { out[j++] = (accum >> 8) & 0xff; }

            if (j < outLen) { out[j++] = accum & 0xff; }



            accum = 0;

        }

    }



    // Check if we exited the loop early.

    if (j < outLen) {

        free(buffer);

        return NULL;

    }



    *outSize = outLen;

    return (uint8_t *)buffer;

}



static void extractAlbumArt(

        const sp<MetaData> &fileMeta, const void *data, size_t size) {

    ALOGV("extractAlbumArt from '%s'", (const char *)data);



    size_t flacSize;

    uint8_t *flac = DecodeBase64((const char *)data, size, &flacSize);



    if (flac == NULL) {

    sp<ABuffer> flacBuffer = decodeBase64(AString((const char *)data, size));

    if (flacBuffer == NULL) {

        ALOGE("malformed base64 encoded data.");

        return;

    }



    size_t flacSize = flacBuffer->size();

    uint8_t *flac = flacBuffer->data();

    ALOGV("got flac of size %zu", flacSize);



    uint32_t picType;
@@ -1298,24 +1224,24 @@ static void extractAlbumArt( 
    char type[128];



    if (flacSize < 8) {

        goto exit;

        return;

    }



    picType = U32_AT(flac);



    if (picType != 3) {

        // This is not a front cover.

        goto exit;

        return;

    }



    typeLen = U32_AT(&flac[4]);

    if (typeLen > sizeof(type) - 1) {

        goto exit;

        return;

    }



    // we've already checked above that flacSize >= 8

    if (flacSize - 8 < typeLen) {

        goto exit;

        return;

    }



    memcpy(type, &flac[8], typeLen);
@@ -1325,7 +1251,7 @@ static void extractAlbumArt( 


    if (!strcmp(type, "-->")) {

        // This is not inline cover art, but an external url instead.

        goto exit;

        return;

    }



    descLen = U32_AT(&flac[8 + typeLen]);
@@ -1333,7 +1259,7 @@ static void extractAlbumArt( 
    if (flacSize < 32 ||

        flacSize - 32 < typeLen ||

        flacSize - 32 - typeLen < descLen) {

        goto exit;

        return;

    }



    dataLen = U32_AT(&flac[8 + typeLen + 4 + descLen + 16]);
@@ -1341,7 +1267,7 @@ static void extractAlbumArt( 


    // we've already checked above that (flacSize - 32 - typeLen - descLen) >= 0

    if (flacSize - 32 - typeLen - descLen < dataLen) {

        goto exit;

        return;

    }



    ALOGV("got image data, %zu trailing bytes",
@@ -1351,10 +1277,6 @@ static void extractAlbumArt( 
            kKeyAlbumArt, 0, &flac[8 + typeLen + 4 + descLen + 20], dataLen);



    fileMeta->setCString(kKeyAlbumArtMIME, type);



exit:

    free(flac);

    flac = NULL;

}



////////////////////////////////////////////////////////////////////////////////

media/libstagefright/foundation/base64.cpp

+8 −3
Original line number Diff line number Diff line
@@ -22,11 +22,11 @@ 
namespace android {



sp<ABuffer> decodeBase64(const AString &s) {

    if ((s.size() % 4) != 0) {

    size_t n = s.size();

    if ((n % 4) != 0) {

        return NULL;

    }



    size_t n = s.size();

    size_t padding = 0;

    if (n >= 1 && s.c_str()[n - 1] == '=') {

        padding = 1;
@@ -40,11 +40,16 @@ sp<ABuffer> decodeBase64(const AString &s) { 
        }

    }



    size_t outLen = 3 * s.size() / 4 - padding;

    // We divide first to avoid overflow. It's OK to do this because we

    // already made sure that n % 4 == 0.

    size_t outLen = (n / 4) * 3 - padding;



    sp<ABuffer> buffer = new ABuffer(outLen);



    uint8_t *out = buffer->data();

    if (out == NULL || buffer->size() < outLen) {

        return NULL;

    }

    size_t j = 0;

    uint32_t accum = 0;

    for (size_t i = 0; i < n; ++i) {