Upcast buffer length comparison to prevent integer wraparound

[Noxwizard]

Most of the readX functions in pe-parser-library/src/buffer.cpp suffer from an integer overflow bug which leads to an out-of-bounds read, which causes a segmentation fault. The user-controlled offset variable should be upcast to a larger type during the computation that checks see if it exceeds the buffer's length. In the provided test case, offset is 0xFFFFFFFF when the segfault occurs.

The following occurs running on Ubuntu 19 with g++ 9.2.1 and sanitizers enabled:

$ ./dump-pe int_wrap_unbound_read.bin
AddressSanitizer:DEADLYSIGNAL
=================================================================
==4356==ERROR: AddressSanitizer: SEGV on unknown address 0x7fb0b0fb0fff (pc 0x7fafb4b63ed8 bp 0x7ffd9e7824c0 sp 0x7ffd9e782320 T0)
==4356==The signal is caused by a READ memory access.
    #0 0x7fafb4b63ed7 in peparse::readQword(peparse::_bounded_buffer*, unsigned int, unsigned long&) /home/user/Desktop/afl/pe-parse/pe-parser-library/src/buffer.cpp:160
    #1 0x7fafb4b91f04 in peparse::getSymbolTable(peparse::_parsed_pe*) /home/user/Desktop/afl/pe-parse/pe-parser-library/src/parse.cpp:2036
    #2 0x7fafb4b98a19 in peparse::ParsePEFromBuffer(peparse::_bounded_buffer*) /home/user/Desktop/afl/pe-parse/pe-parser-library/src/parse.cpp:2429
    #3 0x7fafb4b98e8c in peparse::ParsePEFromFile(char const*) /home/user/Desktop/afl/pe-parse/pe-parser-library/src/parse.cpp:2448
    #4 0x55a6eebc5b72 in main /home/user/Desktop/afl/pe-parse/dump-pe/main.cpp:313
    #5 0x7fafb3d101e2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x271e2)
    #6 0x55a6eebc1e2d in _start (/home/user/Desktop/afl/pe-parse/build/dump-pe/dump-pe+0x44e2d)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /home/user/Desktop/afl/pe-parse/pe-parser-library/src/buffer.cpp:160 in peparse::readQword(peparse::_bounded_buffer*, unsigned int, unsigned long&)
==4356==ABORTING

With the fix:

$ ./dump-pe int_wrap_unbound_read.bin
Error: 9 (Bad magic)
Location: getSymbolTable:2037

Attached is the test case generated by AFL: int_wrap_unbound_read.zip

[CLAassistant]

All committers have signed the CLA.

[woodruffw]

Thanks for the fix!

[woodruffw]

I'll merge this, and also do a follow-up PR that adds your malformed PE as a testcase.

[woodruffw]

This has been cut with v1.3.0.