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

Commit 1784a8db authored by Wonsik Kim's avatar Wonsik Kim Committed by Android (Google) Code Review
Browse files

Merge "DO NOT MERGE Avoid size_t overflow in base64 decoding once again" into klp-dev

parents 69ae6a87 278ed118
Loading
Loading
Loading
Loading
+13 −91
Original line number Diff line number Diff line
@@ -22,6 +22,7 @@

#include <cutils/properties.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>
@@ -826,94 +827,19 @@ 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;
    }

    ALOGV("got flac of size %d", flacSize);
    size_t flacSize = flacBuffer->size();
    uint8_t *flac = flacBuffer->data();
    ALOGV("got flac of size %zu", flacSize);

    uint32_t picType;
    uint32_t typeLen;
@@ -922,24 +848,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);
@@ -949,7 +875,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]);
@@ -957,7 +883,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]);
@@ -965,7 +891,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, %d trailing bytes",
@@ -975,10 +901,6 @@ static void extractAlbumArt(
            kKeyAlbumArt, 0, &flac[8 + typeLen + 4 + descLen + 20], dataLen);

    fileMeta->setCString(kKeyAlbumArtMIME, type);

exit:
    free(flac);
    flac = NULL;
}

////////////////////////////////////////////////////////////////////////////////
+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;
@@ -36,11 +36,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) {