Uploaded image for project: 'Core ReactOS'
  1. Core ReactOS
  2. CORE-10523

Error in parser of PE executables



    • Guilty Revision:


      While playing with ReactOS, I have noticed some interesting bugs:when navigating the files with the Explorer, I noticed that lot of executables did not show the icon.

      You could see for example:

      Killer Loop Demo 1.png = Directory listing on Windows.
      Killer Loop Demo 2.png = Directory listing on ReactOS.

      F197-1.png = Directory listing on Windows.
      F197-2.png = Directory listing on ReactOS.

      As you can see, the two executables killerloop_demo.exe and killerloop_demo_3dfx.exe do not show their icons.
      The same thing happens with f197I.exe and f197d3dI.exe for the demo of the game F1 '97.

      I tried to inspect a bit this defect and I found that the cause of both problems is an error in the parsing of the PE executable sections.

      The various API functions useful for extracting icons or cursors from executables and DLLs made use of some utility functions for parsing the sections of a PE executable.
      One of these functions is RtlImageRvaToSection().
      There are two identical implementation of this function:

      • reactos\lib\rtl\image.c
      • reactos\dll\win32\dbghelp\compat.c

      The current code is using VirtualSize field (and only this one) for selecting the section where the desired virtual address is located, but this is the error.
      The description at:


      says that:
      "VirtualAddress: The total size of the section when loaded into memory, in bytes. If this value is greater than the SizeOfRawData member, the section is filled with zeroes."

      "SizeOfRawData: The size of the initialized data on disk, in bytes. [...] If this value is less than the VirtualSize member, the remainder of the section is filled with zeroes. If the section contains only uninitialized data, the member is zero."

      These statements just mean that SizeOfRawData is the size of the section on Disk, while VirtualSize is the size of the section when loaded into memory.
      This is what I have understood by reading it.
      So, in my opinion SizeOfRawData field prevails the value into VirtualSize: if SizeOfRawData is zero (if we are thinking on an empty section generated at runtime), then VirtualSize can be evaluated instead.
      In the executables without icon displayed I have seen that there are sections with VirtualSize equal to zero.
      Evidently, if you look the current code, if VirtualSize is zero then there is no way to extract the icon from disk because the search will always fail.
      In fact, it does not necessary mean that something on disk must stay also in memory.

      I have modified a bit the two implementation RtlImageRvaToSection(), and now the icons for the executables are shown correctly, see screenshot "Killer Loop Demo 3.png".

      This fix has solved almost all troubles on the missing icons, except the ones of F1 '97 (that's why I have chosen them for this bug report). Here there is surely another problem somewhere in the parsing of the icon group but I have not found it exactly where it is located. The identifier of the icon is zero, that's why the current code does not find it, but at the moment I was not able to find information about handling this particular case.

      I also suggest to retest CORE-9548, perhaps this fix has solved it.



        1. carlo_CORE-10523.patch
          2 kB
        2. CORE-10523.patch
          3 kB
        3. dbghelp.txt
          0.9 kB
        4. exticon.txt
          0.6 kB
        5. F197-1.png
          15 kB
        6. F197-2.png
          23 kB
        7. Killer Loop Demo 1.png
          Killer Loop Demo 1.png
          14 kB
        8. Killer Loop Demo 2.png
          Killer Loop Demo 2.png
          21 kB
        9. Killer Loop Demo 3.png
          Killer Loop Demo 3.png
          21 kB
        10. rtl.txt
          0.8 kB

          Issue Links



              • Assignee:
                hbelusca hbelusca
                Carlo Bramix Carlo Bramix
              • Votes:
                1 Vote for this issue
                5 Start watching this issue


                • Created: