diff --git a/include/retdec/fileformat/file_format/pe/pe_format_parser.h b/include/retdec/fileformat/file_format/pe/pe_format_parser.h index ddf1d8e3f..f3fb21cd6 100644 --- a/include/retdec/fileformat/file_format/pe/pe_format_parser.h +++ b/include/retdec/fileformat/file_format/pe/pe_format_parser.h @@ -265,8 +265,8 @@ class PeFormatParser section.setType(sectionType); section.setIndex(secIndex); section.setOffset(imageLoader.getRealPointerToRawData(secIndex)); - section.setSizeInFile(pSectionHeader->SizeOfRawData); - section.setSizeInMemory(pSectionHeader->VirtualSize); + section.setSizeInFile(imageLoader.getRealSizeOfRawData(secIndex)); + section.setSizeInMemory((pSectionHeader->VirtualSize != 0) ? pSectionHeader->VirtualSize : pSectionHeader->SizeOfRawData); section.setAddress(pSectionHeader->VirtualAddress ? imageLoader.getImageBase() + pSectionHeader->VirtualAddress : 0); section.setMemory(section.getAddress()); section.setPeCoffFlags(pSectionHeader->Characteristics); diff --git a/include/retdec/fileformat/types/resource_table/resource_icon.h b/include/retdec/fileformat/types/resource_table/resource_icon.h index cacddcaa7..8644b2022 100644 --- a/include/retdec/fileformat/types/resource_table/resource_icon.h +++ b/include/retdec/fileformat/types/resource_table/resource_icon.h @@ -56,6 +56,8 @@ class ResourceIcon : public Resource /// @{ bool hasLoadedProperties() const; bool hasValidColorCount() const; + bool hasValidDimensions() const; + /// @} }; diff --git a/include/retdec/pelib/DelayImportDirectory.h b/include/retdec/pelib/DelayImportDirectory.h index 66cfd067f..d986c0ad5 100644 --- a/include/retdec/pelib/DelayImportDirectory.h +++ b/include/retdec/pelib/DelayImportDirectory.h @@ -102,7 +102,7 @@ namespace PeLib return ERROR_INVALID_FILE; init(); - // Keep loading until we encounter an entry filles with zeros + // Keep loading until we encounter an entry filled with zeros for(std::uint32_t i = 0;; i += sizeof(PELIB_IMAGE_DELAY_LOAD_DESCRIPTOR)) { PELIB_IMAGE_DELAY_IMPORT_DIRECTORY_RECORD rec; @@ -113,8 +113,14 @@ namespace PeLib if(!imageLoader.readImage(&rec.delayedImport, rva + i, sizeof(PELIB_IMAGE_DELAY_LOAD_DESCRIPTOR))) break; - // Valid delayed import entry starts either with 0 or 0x01 - if(rec.delayedImport.Attributes & 0xFFFF0000) + // Valid delayed import entry starts either with 0 or 0x01. + // We strict require one of the valid values here + if(rec.delayedImport.Attributes > PELIB_DELAY_ATTRIBUTE_V2) + break; + + // Stop on blatantly invalid entries + if(rec.delayedImport.NameRva < sizeof(PELIB_IMAGE_DOS_HEADER) || + rec.delayedImport.DelayImportNameTableRva < sizeof(PELIB_IMAGE_DOS_HEADER)) break; // Check for the termination entry diff --git a/include/retdec/pelib/ImageLoader.h b/include/retdec/pelib/ImageLoader.h index d6589be2c..f0fe7e01d 100644 --- a/include/retdec/pelib/ImageLoader.h +++ b/include/retdec/pelib/ImageLoader.h @@ -172,6 +172,7 @@ class ImageLoader std::uint32_t vaToRva(std::uint64_t VirtualAddress) const; std::uint32_t getFileOffsetFromRva(std::uint32_t rva) const; std::uint32_t getRealPointerToRawData(std::size_t sectionIndex) const; + std::uint32_t getRealSizeOfRawData(std::size_t sectionIndex) const; std::uint32_t getImageProtection(std::uint32_t characteristics) const; std::size_t getSectionIndexByRva(std::uint32_t Rva) const; diff --git a/src/fileformat/file_format/file_format.cpp b/src/fileformat/file_format/file_format.cpp index 70aeb87a3..18c1daf67 100644 --- a/src/fileformat/file_format/file_format.cpp +++ b/src/fileformat/file_format/file_format.cpp @@ -83,7 +83,7 @@ bool isAddressFromRegion(const SecSeg *actualRegion, const SecSeg *newRegion, st return false; } - unsigned long long newRegionSize; + unsigned long long newRegionSize = 0; if(!newRegion->getSizeInMemory(newRegionSize)) { newRegionSize = newRegion->getSizeInFile(); diff --git a/src/fileformat/types/resource_table/resource_icon.cpp b/src/fileformat/types/resource_table/resource_icon.cpp index c2bf013e0..6b8ea6505 100644 --- a/src/fileformat/types/resource_table/resource_icon.cpp +++ b/src/fileformat/types/resource_table/resource_icon.cpp @@ -179,5 +179,15 @@ bool ResourceIcon::hasValidColorCount() const return validColorCount; } +/** +* Returns tru if the icon dimensions were already set before +* @return @c `true` if they were, otherwise `false` +*/ +bool ResourceIcon::hasValidDimensions() const +{ + return (width && height); +} + + } // namespace fileformat } // namespace retdec diff --git a/src/fileformat/types/resource_table/resource_icon_group.cpp b/src/fileformat/types/resource_table/resource_icon_group.cpp index 9896a8a40..825eda77d 100644 --- a/src/fileformat/types/resource_table/resource_icon_group.cpp +++ b/src/fileformat/types/resource_table/resource_icon_group.cpp @@ -39,27 +39,31 @@ constexpr std::pair iconPriorities[] = bool iconCompare(const retdec::fileformat::ResourceIcon *i1, const retdec::fileformat::ResourceIcon *i2) { auto i1Width = i1->getWidth(); + auto i1Height = i1->getHeight(); auto i1BitCount = i1->getBitCount(); auto i2Width = i2->getWidth(); auto i2Height = i2->getHeight(); auto i2BitCount = i2->getBitCount(); - if(i2Width != i2Height) - { + // Priority 1: icon with the same dimensions over icons + if((i1Width == i1Height) && (i2Width != i2Height)) return false; - } + if((i1Width != i1Height) && (i2Width == i2Height)) + return true; for(const auto &p : iconPriorities) { + // Priority 2: Icons where both size and bit count match if(p.first == i1Width && p.second == i1BitCount) - { return false; - } - if(p.first == i2Width && p.second == i2BitCount) - { return true; - } + + // Priority 3: Icons where size matches + if(p.first == i1Width) + return false; + if(p.first == i2Width) + return true; } return false; @@ -183,8 +187,8 @@ bool ResourceIconGroup::getEntryWidth(std::size_t eIndex, std::uint16_t &width) return false; } - width = bytes[0]; - + if((width = bytes[0]) == 0) + width = 256; // https://devblogs.microsoft.com/oldnewthing/20101018-00/?p=12513 return true; } @@ -203,8 +207,8 @@ bool ResourceIconGroup::getEntryHeight(std::size_t eIndex, std::uint16_t &height return false; } - height = bytes[0]; - + if((height = bytes[0]) == 0) + height = 256; // https://devblogs.microsoft.com/oldnewthing/20101018-00/?p=12513 return true; } diff --git a/src/fileformat/types/resource_table/resource_table.cpp b/src/fileformat/types/resource_table/resource_table.cpp index aab94fb5a..98dacf196 100644 --- a/src/fileformat/types/resource_table/resource_table.cpp +++ b/src/fileformat/types/resource_table/resource_table.cpp @@ -781,6 +781,11 @@ void ResourceTable::linkResourceIconGroups() continue; } + // Multiple icon group may reference an icon. If that happens, do not rewrite + // icon dimensions. Doing so messes up with the icon hash, and we only care for the first icon anyway + if(icon->hasValidDimensions()) + continue; + icon->setWidth(width); icon->setHeight(height); icon->setIconSize(iconSize); diff --git a/src/pelib/ImageLoader.cpp b/src/pelib/ImageLoader.cpp index 6bb7bb866..93c2b68b7 100644 --- a/src/pelib/ImageLoader.cpp +++ b/src/pelib/ImageLoader.cpp @@ -553,6 +553,18 @@ uint32_t PeLib::ImageLoader::getRealPointerToRawData(size_t sectionIndex) const return sections[sectionIndex].PointerToRawData & ~(PELIB_SECTOR_SIZE - 1); } +uint32_t PeLib::ImageLoader::getRealSizeOfRawData(std::size_t sectionIndex) const +{ + if(sectionIndex >= sections.size()) + return UINT32_MAX; + if(optionalHeader.SectionAlignment < PELIB_PAGE_SIZE) + return sections[sectionIndex].SizeOfRawData; + + uint32_t beginOfRawData = sections[sectionIndex].PointerToRawData & ~(PELIB_SECTOR_SIZE - 1); + uint32_t endOfRawData = sections[sectionIndex].PointerToRawData + AlignToSize(sections[sectionIndex].SizeOfRawData, optionalHeader.FileAlignment); + return endOfRawData - beginOfRawData; +} + uint32_t PeLib::ImageLoader::getImageProtection(uint32_t sectionCharacteristics) const { uint32_t Index = 0; diff --git a/src/pelib/ResourceDirectory.cpp b/src/pelib/ResourceDirectory.cpp index cb418bf03..1dbfb4619 100644 --- a/src/pelib/ResourceDirectory.cpp +++ b/src/pelib/ResourceDirectory.cpp @@ -669,8 +669,6 @@ namespace PeLib return ERROR_NONE; } - // Load all entries to the vector - std::vector vResourceChildren(uiNumberOfEntries); resDir->insertNodeOffset(uiOffset); if (uiNumberOfEntries > 0) @@ -678,6 +676,7 @@ namespace PeLib resDir->addOccupiedAddressRange(uiRva, uiRva + uiNumberOfEntries * PELIB_IMAGE_RESOURCE_DIRECTORY_ENTRY::size() - 1); } + // Load all entries to the vector for (unsigned int i = 0; i < uiNumberOfEntries; i++) { ResourceChild rc; @@ -720,7 +719,7 @@ namespace PeLib { // Check whether we have enough space to read at least one character unsigned int uiNameOffset = rc.entry.irde.Name & PELIB_IMAGE_RESOURCE_RVA_MASK; - if (uiRva + uiNameOffset + sizeof(std::uint16_t) > sizeOfImage) + if (uiRsrcRva + uiNameOffset + sizeof(std::uint16_t) > sizeOfImage) { return ERROR_INVALID_FILE; }