Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add check that the resource file offset is valid #982

Merged
merged 5 commits into from
Aug 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/retdec/fileformat/types/resource_table/resource.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ class Resource

/// @name Other methods
/// @{
bool isValidOffset() const;
bool isLoaded() const;
bool hasValidName() const;
bool hasValidId() const;
Expand Down
1 change: 1 addition & 0 deletions include/retdec/pelib/ImageLoader.h
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ class ImageLoader

std::uint32_t vaToRva(std::uint64_t VirtualAddress) const;
std::uint32_t getFileOffsetFromRva(std::uint32_t rva) const;
std::uint32_t getValidOffsetFromRva(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;
Expand Down
2 changes: 1 addition & 1 deletion src/cpdetect/heuristics/pe_heuristics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1813,7 +1813,7 @@ void PeHeuristics::getSevenZipHeuristics()
{
// See: VS_VERSIONINFO structure documentation
auto resource = resourceTable->getResourceWithType(16);
if (resource)
if (resource && resource->isValidOffset())
{
std::uint64_t infoL = 0;
auto offset = resource->getOffset();
Expand Down
8 changes: 4 additions & 4 deletions src/fileformat/file_format/pe/pe_format.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1762,8 +1762,8 @@ void PeFormat::loadResources()
resource->setNameId(nameChild->getOffsetToName());
}

unsigned long long dataOffset;
getOffsetFromAddress(dataOffset, imageBase + lanLeaf->getOffsetToData());
// Check if the offset is actually within the section bounds
std::uint64_t dataOffset = getImageLoader().getValidOffsetFromRva(lanLeaf->getOffsetToData());
resource->setOffset(dataOffset);
resource->setSizeInFile(lanLeaf->getSize());
resource->setLanguage(lanChild->getName());
Expand Down Expand Up @@ -3627,8 +3627,8 @@ void PeFormat::scanForResourceAnomalies()

// scan for resource stretched over multiple sections
unsigned long long resAddr;
if (getAddressFromOffset(resAddr, res->getOffset()) &&
isObjectStretchedOverSections(resAddr, res->getSizeInFile()))
if (res->isValidOffset() && getAddressFromOffset(resAddr, res->getOffset()) &&
isObjectStretchedOverSections(resAddr, res->getSizeInFile()))
{
anomalies.emplace_back("StretchedResource", "Resource " + replaceNonprintableChars(msgName) + " is stretched over multiple sections");
}
Expand Down
11 changes: 11 additions & 0 deletions src/fileformat/types/resource_table/resource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,17 @@ void Resource::setSublanguageId(std::size_t rId)
sublanguageIdIsValid = true;
}

/**
* @brief Checks if the offset is valid, with UINT32_MAX reserved as invalid
*
* @return true
* @return false
*/
bool Resource::isValidOffset() const
{
return offset != UINT32_MAX;
}

/**
* A method which indicates whether resource is loaded.
* @return `true` if it is, otherwise `false`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ std::string ResourceTable::getResourceSublanguageIdStr(std::size_t index, std::i
std::string ResourceTable::getResourceOffsetStr(std::size_t index, std::ios_base &(* format)(std::ios_base &)) const
{
const auto *record = table ? table->getResource(index) : nullptr;
return record ? getNumberAsString(record->getOffset(), format) : "";
return record && record->isValidOffset() ? getNumberAsString(record->getOffset(), format) : "";
}

/**
Expand Down
Loading