erocarrera / pefile

pefile is a Python module to read and work with PE (Portable Executable) files
MIT License
1.86k stars 519 forks source link

Fix undeclared next_section_virtual_address usage #429

Open gdesmar opened 1 week ago

gdesmar commented 1 week ago

The following two files are making pefile crash with the next stacktrace: aksdf.sys hardlock.sys

  File ".../extract.py", line 937, in extract_setup_factory
    pe = pefile.PE(request.file_path, fast_load=True)
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../pefile.py", line 2941, in __init__
    self.__parse__(name, data, fast_load)
  File ".../pefile.py", line 3313, in __parse__
    offset = self.parse_sections(sections_offset)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../pefile.py", line 3656, in parse_sections
    if section.Name.rstrip(b"\x00") == b"PAGE" and self.is_driver():
                                                   ^^^^^^^^^^^^^^^^
  File ".../pefile.py", line 7865, in is_driver
    self.parse_data_directories(
  File ".../pefile.py", line 3768, in parse_data_directories
    value = entry[1](dir_entry.VirtualAddress, dir_entry.Size)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../pefile.py", line 5884, in parse_import_directory
    data = self.get_data(rva, image_import_descriptor_size)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../pefile.py", line 6370, in get_data
    s = self.get_section_by_rva(rva)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../pefile.py", line 6538, in get_section_by_rva
    if section.contains_rva(rva):
       ^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../pefile.py", line 1285, in contains_rva
    self.next_section_virtual_address is not None
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'SectionStructure' object has no attribute 'next_section_virtual_address'

From my understanding, the parse_sections function may end up calling self.is_driver() as it's parsing the sections, but before the parse_sections function goes over the newly parsed sections to configure the next_section_virtual_address variable. Since is_driver() ends up relying on it, it is crashing.

I believe the code that needs the is_driver() call is only adding warnings, so it would not cause any functional change to put it after the code assigning the value to next_section_virtual_address.