search

avast / pelib

PE file manipulation library.
MIT License
63 stars 30 forks source link

Suspicious virtual address arithmetic #1

Closed ladislav-zezula closed 6 years ago

ladislav-zezula commented 6 years ago

In the DelayImportDirectory::read() function (DelayImportDirectory.h), there is a suspicious arithmetic which possibly subtracts a 64-bit image base from a 32-bit value:

    if (std::abs(static_cast<int>(uiDelayImportsVa - rec.NameRva)) <
        std::abs(static_cast<int>(uiDelayImportsRva - rec.NameRva)))
    {
        rec.NameRva -= peHeader.getImageBase();
    }

The meaning of the entire block is a mystery to me (perhaps parsing files that are memory image dumps?). However, it may lead to undefined results, as image bases in 64-bit PE files are also 64-bit and typically exceed the 4 GB boundary (default image base for files produces by MSVC linker is 00000001-40000000).

metthal commented 6 years ago

The mentioned code probably tries to solve this in the very hacky way. We should use the attribute mentioned in the SO answer to recognize whether delayed import directory contains VAs or RVAs.

s3rvac commented 6 years ago

Possibly related to https://github.com/avast-tl/retdec/issues/282.

ladislav-zezula commented 6 years ago

Fixed in commit ba7126dc13bd378c5360b68b0141488d5ce18d52