Lz retdec 54 (#981)

* Added more checks for delay import directory

* Hardened resilience against malformed delayed import directories

* Removed unnecessary file

* Iconhash problem fixed

* Fixed getRealSizeOfRawData()

Co-authored-by: Ladislav Zezula <ladislav.zezula@avast.com>
This commit is contained in:
Ladislav Zezula 2021-07-21 11:14:56 +02:00 committed by GitHub
parent 21ad19c2ca
commit d3677c292d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 60 additions and 21 deletions

View File

@ -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);

View File

@ -56,6 +56,8 @@ class ResourceIcon : public Resource
/// @{
bool hasLoadedProperties() const;
bool hasValidColorCount() const;
bool hasValidDimensions() const;
/// @}
};

View File

@ -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

View File

@ -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;

View File

@ -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();

View File

@ -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

View File

@ -39,27 +39,31 @@ constexpr std::pair<uint16_t, uint16_t> 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;
}

View File

@ -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);

View File

@ -556,6 +556,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;

View File

@ -677,8 +677,6 @@ namespace PeLib
return ERROR_NONE;
}
// Load all entries to the vector
std::vector<PELIB_IMAGE_RESOURCE_DIRECTORY_ENTRY> vResourceChildren(uiNumberOfEntries);
resDir->insertNodeOffset(uiOffset);
if (uiNumberOfEntries > 0)
@ -686,6 +684,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;
@ -728,7 +727,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;
}