Revert "Avoid size_t overflow in base64 decoding once again"
This reverts commit c9ac5dfdafed1c66beae090cafa97002764e0ca3.
Change-Id: Iae9707bbd8641a0bb00fcda39a20eb8b8f4f5232
diff --git a/media/libstagefright/OggExtractor.cpp b/media/libstagefright/OggExtractor.cpp
index 10dce3b..130f5a5 100644
--- a/media/libstagefright/OggExtractor.cpp
+++ b/media/libstagefright/OggExtractor.cpp
@@ -22,7 +22,6 @@
#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>
@@ -831,18 +830,93 @@
}
+// 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);
- sp<ABuffer> flacBuffer = decodeBase64(AString((const char *)data, size));
- if (flacBuffer == NULL) {
+ size_t flacSize;
+ uint8_t *flac = DecodeBase64((const char *)data, size, &flacSize);
+
+ if (flac == 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;
@@ -852,24 +926,24 @@
char type[128];
if (flacSize < 8) {
- return;
+ goto exit;
}
picType = U32_AT(flac);
if (picType != 3) {
// This is not a front cover.
- return;
+ goto exit;
}
typeLen = U32_AT(&flac[4]);
if (typeLen > sizeof(type) - 1) {
- return;
+ goto exit;
}
// we've already checked above that flacSize >= 8
if (flacSize - 8 < typeLen) {
- return;
+ goto exit;
}
memcpy(type, &flac[8], typeLen);
@@ -879,7 +953,7 @@
if (!strcmp(type, "-->")) {
// This is not inline cover art, but an external url instead.
- return;
+ goto exit;
}
descLen = U32_AT(&flac[8 + typeLen]);
@@ -887,7 +961,7 @@
if (flacSize < 32 ||
flacSize - 32 < typeLen ||
flacSize - 32 - typeLen < descLen) {
- return;
+ goto exit;
}
dataLen = U32_AT(&flac[8 + typeLen + 4 + descLen + 16]);
@@ -895,7 +969,7 @@
// we've already checked above that (flacSize - 32 - typeLen - descLen) >= 0
if (flacSize - 32 - typeLen - descLen < dataLen) {
- return;
+ goto exit;
}
ALOGV("got image data, %zu trailing bytes",
@@ -905,6 +979,10 @@
kKeyAlbumArt, 0, &flac[8 + typeLen + 4 + descLen + 20], dataLen);
fileMeta->setCString(kKeyAlbumArtMIME, type);
+
+exit:
+ free(flac);
+ flac = NULL;
}
////////////////////////////////////////////////////////////////////////////////
diff --git a/media/libstagefright/foundation/base64.cpp b/media/libstagefright/foundation/base64.cpp
index 7da7db9..dcf5bef 100644
--- a/media/libstagefright/foundation/base64.cpp
+++ b/media/libstagefright/foundation/base64.cpp
@@ -22,11 +22,11 @@
namespace android {
sp<ABuffer> decodeBase64(const AString &s) {
- size_t n = s.size();
- if ((n % 4) != 0) {
+ if ((s.size() % 4) != 0) {
return NULL;
}
+ size_t n = s.size();
size_t padding = 0;
if (n >= 1 && s.c_str()[n - 1] == '=') {
padding = 1;
@@ -40,16 +40,11 @@
}
}
- // 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;
+ size_t outLen = 3 * s.size() / 4 - 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) {
