search

VirusTotal / yara

The pattern matching swiss knife
https://virustotal.github.io/yara/
BSD 3-Clause "New" or "Revised" License
8.08k stars 1.43k forks source link

Fix imphash issue with empty import names. #1944

Closed wxsBSD closed 1 year ago

wxsBSD commented 1 year ago

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 #1943

plusvic commented 1 year ago

The file tests/data/tiny_empty_import_name wasn't added in the PR. Interesting enough, the tests are failing with a memory leak that doesn't seem related to the change done here.

wxsBSD commented 1 year ago

Oops, I forgot to add it! Will do so as soon as I'm able.

wxsBSD commented 1 year ago

Please don't merge this yet. There are other problems raised in the linked issue that are being looked into.

wxsBSD commented 1 year ago

The file tests/data/tiny_empty_import_name wasn't added in the PR. Interesting enough, the tests are failing with a memory leak that doesn't seem related to the change done here.

Actually, the memory leak was because of my changes. I was not calling yr_free() if the import had an invalid name.

I also discovered that if we failed to allocate a new IMPORT_FUNCTION we would free the name (correctly) but then continue without incrementing the thunk pointer or function index, which would cause an infinite loop. I've fixed that too.

I think this can be reviewed and merged now.