Skip to content

Commit 9de19ea

Browse files
committed
fileformat/resource_table: add more checks to icon resource processing.
This fixes and prevents segfaults in PE icon resource parsing and hash computation. This fixes internal issue ODIN-109.
1 parent 0a67335 commit 9de19ea

File tree

2 files changed

+50
-9
lines changed

2 files changed

+50
-9
lines changed

include/retdec/fileformat/types/resource_table/bitmap_image.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,14 @@ namespace fileformat {
1919
struct BitmapInformationHeader
2020
{
2121
std::uint32_t size; ///< size of header (must be 40)
22-
std::int32_t width; ///< width (signed)
23-
std::int32_t height; ///< height (signed)
22+
std::uint32_t width; ///< width
23+
std::uint32_t height; ///< height
2424
std::uint16_t planes; ///< color planes (must be 1)
2525
std::uint16_t bitCount; ///< bpp color depth
2626
std::uint32_t compression; ///< compression
2727
std::uint32_t bitmapSize; ///< size of bitmap
28-
std::int32_t horizontalRes; ///< horizontal resolution (signed, hint for BMP reader)
29-
std::int32_t verticalRes; ///< vertical resolution (signed, hint for BMP reader)
28+
std::uint32_t horizontalRes; ///< horizontal resolution (hint for BMP reader)
29+
std::uint32_t verticalRes; ///< vertical resolution (hint for BMP reader)
3030
std::uint32_t colorsUsed; ///< number of colors in the image
3131
std::uint32_t colorImportant; ///< minimal number of important colors (generaly ignored)
3232

src/fileformat/types/resource_table/bitmap_image.cpp

Lines changed: 46 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -120,14 +120,14 @@ bool BitmapImage::parseDibHeader(const ResourceIcon &icon, struct BitmapInformat
120120

121121
std::size_t offset = 0;
122122
res.size = *reinterpret_cast<uint32_t *>(&bytes.data()[offset]); offset += sizeof(res.size);
123-
res.width = *reinterpret_cast<int32_t *>(&bytes.data()[offset]); offset += sizeof(res.width);
124-
res.height = *reinterpret_cast<int32_t *>(&bytes.data()[offset]); offset += sizeof(res.height);
123+
res.width = *reinterpret_cast<uint32_t *>(&bytes.data()[offset]); offset += sizeof(res.width);
124+
res.height = *reinterpret_cast<uint32_t *>(&bytes.data()[offset]); offset += sizeof(res.height);
125125
res.planes = *reinterpret_cast<uint16_t *>(&bytes.data()[offset]); offset += sizeof(res.planes);
126126
res.bitCount = *reinterpret_cast<uint16_t *>(&bytes.data()[offset]); offset += sizeof(res.bitCount);
127127
res.compression = *reinterpret_cast<uint32_t *>(&bytes.data()[offset]); offset += sizeof(res.compression);
128128
res.bitmapSize = *reinterpret_cast<uint32_t *>(&bytes.data()[offset]); offset += sizeof(res.bitmapSize);
129-
res.horizontalRes = *reinterpret_cast<int32_t *>(&bytes.data()[offset]); offset += sizeof(res.horizontalRes);
130-
res.verticalRes = *reinterpret_cast<int32_t *>(&bytes.data()[offset]); offset += sizeof(res.verticalRes);
129+
res.horizontalRes = *reinterpret_cast<uint32_t *>(&bytes.data()[offset]); offset += sizeof(res.horizontalRes);
130+
res.verticalRes = *reinterpret_cast<uint32_t *>(&bytes.data()[offset]); offset += sizeof(res.verticalRes);
131131
res.colorsUsed = *reinterpret_cast<uint32_t *>(&bytes.data()[offset]); offset += sizeof(res.colorsUsed);
132132
res.colorImportant = *reinterpret_cast<uint32_t *>(&bytes.data()[offset]); offset += sizeof(res.colorImportant);
133133

@@ -147,7 +147,8 @@ bool BitmapImage::parseDibHeader(const ResourceIcon &icon, struct BitmapInformat
147147
}
148148

149149
if (res.size != res.headerSize() || res.planes != 1 || res.compression != 0 ||
150-
res.width * 2 != res.height || res.width > 512 || res.bitCount > 32)
150+
res.width * 2 != res.height || res.width > 512 || res.height > 1024 ||
151+
res.bitCount > 32)
151152
{
152153
return false;
153154
}
@@ -209,6 +210,11 @@ bool BitmapImage::parseDib1Data(const ResourceIcon &icon, const struct BitmapInf
209210
{
210211
for (std::size_t i = 0; i < 8; i++)
211212
{
213+
if (bytes.size() <= offset)
214+
{
215+
return false;
216+
}
217+
212218
auto bit = (bytes[offset] & (0x01 << (7 - i)));
213219
auto index = (bit == 0) ? 0 : 1;
214220
row.push_back(palette[index]);
@@ -223,6 +229,11 @@ bool BitmapImage::parseDib1Data(const ResourceIcon &icon, const struct BitmapInf
223229
{
224230
for (std::size_t i = 0; i < rest; i++)
225231
{
232+
if (bytes.size() <= offset)
233+
{
234+
return false;
235+
}
236+
226237
auto index = !!(bytes[offset] & (0x01 << (7 - i)));
227238
row.push_back(palette[index]);
228239
}
@@ -289,13 +300,23 @@ bool BitmapImage::parseDib4Data(const ResourceIcon &icon, const struct BitmapInf
289300

290301
for (std::size_t j = 0; j < nColumns / 2; j++)
291302
{
303+
if (bytes.size() <= offset)
304+
{
305+
return false;
306+
}
307+
292308
row.push_back(palette[bytes[offset] >> 4]);
293309
row.push_back(palette[bytes[offset] & 0x0F]);
294310
offset++;
295311
}
296312

297313
if (nColumns % 2)
298314
{
315+
if (bytes.size() <= offset)
316+
{
317+
return false;
318+
}
319+
299320
row.push_back(palette[bytes[offset] >> 4]);
300321
offset++;
301322
}
@@ -360,6 +381,11 @@ bool BitmapImage::parseDib8Data(const ResourceIcon &icon, const struct BitmapInf
360381

361382
for (std::size_t j = 0; j < nColumns; j++)
362383
{
384+
if (bytes.size() <= offset)
385+
{
386+
return false;
387+
}
388+
363389
row.push_back(palette[bytes[offset]]);
364390
offset += bytesPP;
365391
}
@@ -409,6 +435,11 @@ bool BitmapImage::parseDib24Data(const ResourceIcon &icon, const struct BitmapIn
409435

410436
for (std::size_t j = 0; j < nColumns; j++)
411437
{
438+
if (bytes.size() <= (offset+2))
439+
{
440+
return false;
441+
}
442+
412443
row.emplace_back(bytes[offset + 2], bytes[offset + 1], bytes[offset], 0xFF);
413444
offset += bytesPP;
414445
}
@@ -457,6 +488,11 @@ bool BitmapImage::parseDib32Data(const ResourceIcon &icon, const struct BitmapIn
457488

458489
for (std::size_t j = 0; j < nColumns; j++)
459490
{
491+
if (bytes.size() <= (offset+3))
492+
{
493+
return false;
494+
}
495+
460496
row.emplace_back(bytes[offset + 2], bytes[offset + 1], bytes[offset], bytes[offset + 3]);
461497
offset += bytesPP;
462498
}
@@ -488,6 +524,11 @@ bool BitmapImage::parseDibPalette(const ResourceIcon &icon, std::vector<struct B
488524

489525
for (std::uint32_t i = 0; i < nBytes; i += 4)
490526
{
527+
if (bytes.size() <= (i+3))
528+
{
529+
return false;
530+
}
531+
491532
palette.emplace_back(bytes[i + 2], bytes[i + 1], bytes[i], bytes[i + 3]);
492533
}
493534

0 commit comments

Comments
 (0)