Skip to content

Commit

Permalink
Fix imphash issue with empty import names.
Browse files Browse the repository at this point in the history
If an import has an empty name skip processing it. This is consistent with the
behavior of pefile (https://github.com/erocarrera/pefile/blob/593d094e35198dad92aaf040bef17eb800c8a373/pefile.py#L5871-L5872).

Add a test case which is just the tiny test file with the first import name set
to all NULL bytes. I tested that pefile calculates the imphash of it, which
matches what YARA now calculates too:

>>> import pefile
>>> pe = pefile.PE('/Users/wxs/src/yara/tests/data/tiny_empty_import_name')
>>> pe.get_imphash()
'0eff3a0eb037af8c1ef0bada984d6af5'
>>>

Fixes VirusTotal#1943
  • Loading branch information
wxsBSD committed Aug 9, 2023
1 parent 3c928f8 commit 5b37105
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 3 deletions.
5 changes: 2 additions & 3 deletions libyara/modules/pe/pe.c
Original file line number Diff line number Diff line change
Expand Up @@ -871,7 +871,7 @@ static IMPORT_FUNCTION* pe_parse_import_descriptor(
rva_address = yr_le32toh(import_descriptor->FirstThunk) +
(sizeof(uint64_t) * func_idx);

if (name != NULL || has_ordinal == 1)
if ((name != NULL && strcmp(name, "")) || has_ordinal == 1)
{
IMPORT_FUNCTION* imported_func = (IMPORT_FUNCTION*) yr_calloc(
1, sizeof(IMPORT_FUNCTION));
Expand Down Expand Up @@ -946,7 +946,7 @@ static IMPORT_FUNCTION* pe_parse_import_descriptor(
rva_address = yr_le32toh(import_descriptor->FirstThunk) +
(sizeof(uint32_t) * func_idx);

if (name != NULL || has_ordinal == 1)
if ((name != NULL && strcmp(name, "")) || has_ordinal == 1)
{
IMPORT_FUNCTION* imported_func = (IMPORT_FUNCTION*) yr_calloc(
1, sizeof(IMPORT_FUNCTION));
Expand Down Expand Up @@ -1127,7 +1127,6 @@ static IMPORTED_DLL* pe_parse_imports(PE* pe)
if (functions != NULL)
{
imported_dll->name = yr_strdup(dll_name);
;
imported_dll->functions = functions;
imported_dll->next = NULL;

Expand Down
10 changes: 10 additions & 0 deletions tests/test-pe.c
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,16 @@ int main(int argc, char** argv)
}",
"tests/data/tiny");

// Make sure imports with no ordinal and an empty name are skipped. This is
// consistent with the behavior of pefile.
assert_true_rule_file(
"import \"pe\" \
rule test { \
condition: \
pe.imphash() == \"0eff3a0eb037af8c1ef0bada984d6af5\" \
}",
"tests/data/tiny_empty_import_name");

#endif

#if defined(HAVE_LIBCRYPTO)
Expand Down

0 comments on commit 5b37105

Please sign in to comment.