microsoft / microsoft/Detours

Bug: DetourOpenBinary doesn't accept valid PEs because of incorrect limit on sections

Open
#268 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
C++
Stars
6.4k
Forks
1.2k
PR merge metrics
No merged PRs in 30d

Description

Bug Description:
Detours allocates IMAGE_NUMBEROF_DIRECTORY_ENTRIES (i.e., 16) entries in the m_SectionHeaders array (https://github.com/microsoft/Detours/blob/master/src/image.cpp#L260). 16 is the correct limit for the DataDirectory[] member of the IMAGE_OPTIONAL_HEADERs. However, Microsoft documentation specifies that the Windows loader can accept up to 96 sections (https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#coff-file-header-object-and-image). I believe this discrepancy is a bug, and it is causing some real world issues with DetourOpenBinary.

PEs compiled with mingw will contain DWARF sections unless the user strips the binary. In practice, there appear to be 8 DWARF sections included by mingw. For x64 binaries, there are also the .pdata and .xdata sections for exception information. On x64 PE binaries compiled with mingw gcc 7.3, there are 17 sections by default. As a result, when DetourOpenBinary is called, it fails in CImage::Read (https://github.com/microsoft/Detours/blob/master/src/image.cpp#L1088).

While stripping the DWARF sections from the binary is a workaround, I believe the appropriate solution is to bring the Detours sections limit in line with published Microsoft documentation and allow up to 96 sections. Looking through winnt.h there doesn't appear to be a good macro for this. Is anyone aware of an existing variable/macro that could be used for this instead of a magic number?

Command-line test case

C:\> x86_64-w64-mingw-gcc helloworld.c -o helloworld.exe
C:\> x86_64-w64-mingw-gcc -shared library.c -o library.dll
C:\> setdll64.exe /d:library.dll helloworld.exe
Adding library.dll to binary files.
  helloworld.exe:
DetourBinaryOpen failed: 192

C:\> x86_64-w64-mingw-strip helloworld.exe
C:\> setdll64.exe /d:library.dll helloworld.exe
Adding library.dll to binary files.
  helloworld.exe:
    library.dll
    KERNEL32.dll -> KERNEL32.dll

Detours version
Version 4.0.1 of Detours (https://github.com/microsoft/Detours/commit/734ac64)

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/image.cpp at the m_SectionHeaders allocation around line 260 and the failure in CImage::Read around line 1088. Reproduce the issue with the provided MinGW x64 helloworld.exe and setdll64.exe command sequence, then verify that an unstripped PE with more than 16 sections succeeds while the stripped-binary behavior remains intact.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
operating-systems
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.