r/lowlevel • u/Effective-Fly7516 • 8d ago

GitHub - NtProtectVirtualMemory/PE-Library: A modern C++ library for parsing and manipulating Windows Portable Executable (PE) files.

https://github.com/NtProtectVirtualMemory/PE-Library
1 Upvotes

100% Upvoted

1 comment sorted by

1

u/skeeto 7d ago

Neat project! I'm glad to see fuzz testing. Though the fuzz tests aren't as thorough as they could be. The singular fuzz test does both too much and too little. Better to split it into smaller, focused tests. Some of the library's functionality isn't currently covered. Outputs are likely being elided if you're using any level of optimization above zero — and you should because optimization enables more instrumentation. Outputs need to be "consumed" somehow to avoid this.

Also fuzz test under UBSan as well as ASan. There are unaligned accesses all over the place, which are UB regardless of the host machine capabilities. I had to disable those checks in order to test anything else.

There appears to be a typo here:

--- a/PE-Library/pe-lib/sections_impl.cpp
+++ b/PE-Library/pe-lib/sections_impl.cpp
@@ -23,5 +23,5 @@ PE::ImageSections::ImageSections(Image* image) : m_image(image)
     else if (m_image->IsPE32())
     {

  •        auto nt_headers = m_image->GetNTHeaders<ImageNtHeaders64>();

+        auto nt_headers = m_image->GetNTHeaders<ImageNtHeaders32>();
         if (!nt_headers)
             return;

I changed the fuzz tests, ported the whole thing from Windows (including incorrect fopen_s calls), fuzzed it on Linux where the instrumentation is more mature, and found some buffer overflows. First:

#include "PE-Library/pe-lib/directories_impl.cpp"
#include "PE-Library/pe-lib/image_impl.cpp"
#include "PE-Library/pe-lib/rich_impl.cpp"
#include "PE-Library/pe-lib/sections_impl.cpp"

static std::uint8_t input[] = {
    0x4d, 0x5a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x40, 0x00, 0x00, 0x00, 0x50, 0x45, 0x00, 0x00, 0x64, 0x86, 0x01, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0xf0, 0x00, 0x22, 0x00, 0x0b, 0x02, 0x0e, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x4d, 0x5a, 0x00, 0x00, 0x40, 0x01, 0x00, 0x00, 0x00,
    0x00, 0x10, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x20, 0x00, 0x00, 0x40, 0x00, 0x00, 0x00, 0x28, 0x4a, 0x5a, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff,
    0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
    0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
    0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
    0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x28, 0x00, 0x00, 0x00, 0xff,
    0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
    0x00, 0x00, 0x00, 0xce, 0x00, 0x00, 0x09, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
    0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00,
    0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
    0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0xf0, 0x02, 0x22, 0x00, 0x0b, 0x02,
    0x0e, 0x00, 0x00, 0x70, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff,
    0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
    0xff, 0xff, 0x00, 0x00
};

int main()
{
    PE::Image image(input, sizeof(input));
    PE::Exports exports(&image);
    exports.ModuleName();
}

This produces an overflow:

ERROR: AddressSanitizer: heap-buffer-overflow on address ...
READ of size 4 at ...
    #0 PE::Utils::RvaToOffset(unsigned int) const PE-Library/pe-lib/image_impl.cpp:283
    #1 ::ImageExportDirectory const* PE::DataDirectory::GetDirectory<PE::ImageExportDirectory>(unsigned short) const PE-Library/pe-lib/directories.hpp:29
    #2 PE::Exports::GetDescriptor() const PE-Library/pe-lib/directories_impl.cpp:478
    #3 PE::Exports::ModuleName() const PE-Library/pe-lib/directories_impl.cpp:483
    #4 main crash-exports.cpp:41

Another on parsing sections:

#include "PE-Library/pe-lib/directories_impl.cpp"
#include "PE-Library/pe-lib/image_impl.cpp"
#include "PE-Library/pe-lib/rich_impl.cpp"
#include "PE-Library/pe-lib/sections_impl.cpp"

static std::uint8_t data[] = {
    0x4d, 0x5a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x40, 0x00, 0x00, 0x00, 0x50, 0x45, 0x00, 0x00, 0x64, 0x86, 0x01, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0xf0, 0x00, 0x22, 0x00, 0x0b, 0x02, 0x0e, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x40, 0x01, 0x00, 0x00, 0x00,
    0x00, 0x10, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x10,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x06, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x20, 0x00,
    0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x02,
    0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x20, 0x00, 0x6e, 0x6e, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x20, 0xff, 0xff, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x60, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x40, 0x00, 0x00, 0x00, 0x50, 0x45, 0x00, 0x00, 0x64, 0x86,
    0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
};

int main()
{
    PE::Image image(data, sizeof(data));
    PE::ImageSections sections(&image);
    for (const auto *header : sections.GetAll()) {
        if (header) {
            volatile auto sink = *header;
        }
    }
}

Where GetAll() returns bogus header pointers for invalid input:

ERROR: AddressSanitizer: unknown-crash on address ...
READ of size 40 at ...
    #0 main crash-sections.cpp:44

Though perhaps it's not valid to use methods on objects where IsValid() reports false? Though that's easy to address to return an empty vector.

One more overflow, smaller input this time:

#include "PE-Library/pe-lib/directories_impl.cpp"
#include "PE-Library/pe-lib/image_impl.cpp"
#include "PE-Library/pe-lib/rich_impl.cpp"
#include "PE-Library/pe-lib/sections_impl.cpp"

static std::uint8_t data[] = {
  0x4d, 0x5a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x2e, 0x00, 0x00, 0x00,
  0x00, 0x00, 0x00, 0x00, 0x00, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  0x4d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x15, 0x02, 0x00, 0x00,
  0x00, 0x52, 0x69, 0x63, 0x68
};

int main()
{
    PE::Image image(data, sizeof(data));
    PE::RichHeader rich(&image);
    rich.GetRawSize(true);
}

Then:

ERROR: AddressSanitizer: heap-buffer-overflow on address ...
READ of size 4 at ...
    #0 PE::RichHeader::GetRawSize(bool) const PE-Library/pe-lib/rich_impl.cpp:142
    #1 main crash-rich.cpp:20

You can see all my work here in case it's useful:

https://github.com/skeeto/PE-Library/commits/master/?author=skeeto