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

fileformat/elf: Prevent creation of duplicate imports while parsing #1210

Merged
merged 2 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion include/retdec/fileformat/types/import_table/import.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class Import

/// @name Getters
/// @{
std::string getName() const;
const std::string& getName() const;
std::uint64_t getLibraryIndex() const;
std::uint64_t getAddress() const;
bool getOrdinalNumber(std::uint64_t &importOrdinalNumber) const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class ImportTable
virtual void computeHashes();
void clear();
void addLibrary(std::string name, bool missingDependency = false);
void addImport(std::unique_ptr<Import>&& import);
const Import* addImport(std::unique_ptr<Import>&& import);
bool hasLibraries() const;
bool hasLibrary(const std::string &name) const;
bool hasLibraryCaseInsensitive(const std::string &name) const;
Expand Down
31 changes: 28 additions & 3 deletions src/fileformat/file_format/elf/elf_format.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

#include <elfio/elf_types.hpp>
#include <functional>
#include <map>
#include <regex>

Expand Down Expand Up @@ -1744,10 +1745,18 @@ void ElfFormat::loadSymbols(const ELFIO::elfio *file, const ELFIO::symbol_sectio
telfhashSymbols = {};
}

// Cache keeping track of created imports (i.e. (name, address) pairs)
// to prevent repeated creation of the same imports.
// To save space, this uses string references and relies on imports not
// being moved -- they should not be, as ImportTable stores vector of unique
// pointers, but if that ever changes, this will end badly.
std::set<std::pair<const std::string&, unsigned long long>> createdImports;
PeterMatula marked this conversation as resolved.
Show resolved Hide resolved

for(std::size_t i = 0, e = elfSymbolTable->get_loaded_symbols_num(); i < e; ++i)
{
auto symbol = std::make_shared<ElfSymbol>();
elfSymbolTable->get_symbol(i, name, value, size, bind, type, link, other);

size ? symbol->setSize(size) : symbol->invalidateSize();
symbol->setType(getSymbolType(bind, type, link));
symbol->setUsageType(getSymbolUsageType(type));
Expand Down Expand Up @@ -1781,23 +1790,39 @@ void ElfFormat::loadSymbols(const ELFIO::elfio *file, const ELFIO::symbol_sectio
importTable = new ElfImportTable();
}
auto keyIter = importNameAddressMap.equal_range(name);

// we create std::set from std::multimap values in order to ensure determinism
std::set<std::pair<std::string, unsigned long long>> addresses(keyIter.first, keyIter.second);
std::set<std::pair<std::string, unsigned long long>> addresses;
for (auto it = keyIter.first; it != keyIter.second; ++it)
{
if (createdImports.count({std::cref(it->first), it->second})) {
addresses.insert(*it);
}
}

for(const auto &address : addresses)
{
auto import = std::make_unique<Import>();
import->setName(name);
import->setAddress(address.second);
import->setUsageType(symbolToImportUsage(symbol->getUsageType()));
importTable->addImport(std::move(import));
auto* inserted = importTable->addImport(std::move(import));

if (inserted) {
createdImports.emplace(inserted->getName(), inserted->getAddress());
}
}
if(keyIter.first == keyIter.second && getSectionFromAddress(value))
{
auto import = std::make_unique<Import>();
import->setName(name);
import->setAddress(value);
import->setUsageType(symbolToImportUsage(symbol->getUsageType()));
importTable->addImport(std::move(import));
auto* inserted = importTable->addImport(std::move(import));

if (inserted) {
createdImports.emplace(inserted->getName(), inserted->getAddress());
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/fileformat/types/import_table/import.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace fileformat {
* Get import name
* @return Import name
*/
std::string Import::getName() const
const std::string& Import::getName() const
{
return name;
}
Expand Down
3 changes: 2 additions & 1 deletion src/fileformat/types/import_table/import_table.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -322,9 +322,10 @@ void ImportTable::addLibrary(std::string name, bool isMissingDependency)
* Add import
* @param import Import which will be added
*/
void ImportTable::addImport(std::unique_ptr<Import>&& import)
const Import* ImportTable::addImport(std::unique_ptr<Import>&& import)
{
imports.push_back(std::move(import));
return imports.back().get();
}

/**
Expand Down
Loading