AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

ICO: Prevent calling 'new' with large values

Browse Source 
numImages is read from untrusted data, and as an unsigned short could
be up to 65,536. Avoid calling new with this number, which could result
in a crash if it pushes the device over the memory limit.

Change-Id: Ifff9e0ac6ade2b3d8584af656ea7d2f9eb4998e2
Reviewed-on: https://skia-review.googlesource.com/21269
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
This commit is contained in:
Leon Scroggins III 2017-06-29 15:41:32 -04:00 committed by Skia Commit-Bot
parent ec25768641
commit 005a970eb9
1 changed files with 22 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
src/codec/SkIcoCodec.cpp
View File

@ -54,14 +54,6 @@ SkCodec* SkIcoCodec::NewFromStream(SkStream* stream) {
return nullptr; return nullptr;
} }
// Ensure that we can read all of indicated directory entries
std::unique_ptr<uint8_t[]> entryBuffer(new uint8_t[numImages * kIcoDirEntryBytes]);
if (inputStream.get()->read(entryBuffer.get(), numImages*kIcoDirEntryBytes) !=
numImages*kIcoDirEntryBytes) {
SkCodecPrintf("Error: unable to read ico directory entries.\n");
return nullptr;
}
// This structure is used to represent the vital information about entries // This structure is used to represent the vital information about entries
// in the directory header. We will obtain this information for each // in the directory header. We will obtain this information for each
// directory entry. // directory entry.
@ -69,10 +61,24 @@ SkCodec* SkIcoCodec::NewFromStream(SkStream* stream) {
uint32_t offset; uint32_t offset;
uint32_t size; uint32_t size;
}; };
std::unique_ptr<Entry[]> directoryEntries(new Entry[numImages]); SkAutoFree dirEntryBuffer(sk_malloc_flags(sizeof(Entry) * numImages,
SK_MALLOC_TEMP));
if (!dirEntryBuffer) {
SkCodecPrintf("Error: OOM allocating ICO directory for %i images.\n",
numImages);
return nullptr;
}
auto* directoryEntries = reinterpret_cast<Entry*>(dirEntryBuffer.get());
// Iterate over directory entries // Iterate over directory entries
for (uint32_t i = 0; i < numImages; i++) { for (uint32_t i = 0; i < numImages; i++) {
uint8_t entryBuffer[kIcoDirEntryBytes];
if (inputStream->read(entryBuffer, kIcoDirEntryBytes) !=
kIcoDirEntryBytes) {
SkCodecPrintf("Error: Dir entries truncated in ico.\n");
return nullptr;
}
// The directory entry contains information such as width, height, // The directory entry contains information such as width, height,
// bits per pixel, and number of colors in the color palette. We will // bits per pixel, and number of colors in the color palette. We will
// ignore these fields since they are repeated in the header of the // ignore these fields since they are repeated in the header of the
@ -80,16 +86,16 @@ SkCodec* SkIcoCodec::NewFromStream(SkStream* stream) {
// defer to the value in the embedded header anyway. // defer to the value in the embedded header anyway.
// Specifies the size of the embedded image, including the header // Specifies the size of the embedded image, including the header
uint32_t size = get_int(entryBuffer.get(), 8 + i*kIcoDirEntryBytes); uint32_t size = get_int(entryBuffer, 8);
// Specifies the offset of the embedded image from the start of file. // Specifies the offset of the embedded image from the start of file.
// It does not indicate the start of the pixel data, but rather the // It does not indicate the start of the pixel data, but rather the
// start of the embedded image header. // start of the embedded image header.
uint32_t offset = get_int(entryBuffer.get(), 12 + i*kIcoDirEntryBytes); uint32_t offset = get_int(entryBuffer, 12);
// Save the vital fields // Save the vital fields
directoryEntries.get()[i].offset = offset; directoryEntries[i].offset = offset;
directoryEntries.get()[i].size = size; directoryEntries[i].size = size;
} }
// It is "customary" that the embedded images will be stored in order of // It is "customary" that the embedded images will be stored in order of
@ -102,16 +108,15 @@ SkCodec* SkIcoCodec::NewFromStream(SkStream* stream) {
} }
}; };
EntryLessThan lessThan; EntryLessThan lessThan;
SkTQSort(directoryEntries.get(), directoryEntries.get() + numImages - 1, SkTQSort(directoryEntries, &directoryEntries[numImages - 1], lessThan);
lessThan);
// Now will construct a candidate codec for each of the embedded images // Now will construct a candidate codec for each of the embedded images
uint32_t bytesRead = kIcoDirectoryBytes + numImages * kIcoDirEntryBytes; uint32_t bytesRead = kIcoDirectoryBytes + numImages * kIcoDirEntryBytes;
std::unique_ptr<SkTArray<std::unique_ptr<SkCodec>, true>> codecs( std::unique_ptr<SkTArray<std::unique_ptr<SkCodec>, true>> codecs(
new (SkTArray<std::unique_ptr<SkCodec>, true>)(numImages)); new (SkTArray<std::unique_ptr<SkCodec>, true>)(numImages));
for (uint32_t i = 0; i < numImages; i++) { for (uint32_t i = 0; i < numImages; i++) {
uint32_t offset = directoryEntries.get()[i].offset; uint32_t offset = directoryEntries[i].offset;
uint32_t size = directoryEntries.get()[i].size; uint32_t size = directoryEntries[i].size;
// Ensure that the offset is valid // Ensure that the offset is valid
if (offset < bytesRead) { if (offset < bytesRead) {