Bug and Fix: Adding exception stack trace debug to Exe results in invalid PE format, SignTool fails with invalid file

[eivindbakkestuen]

The following info and fix is produced by Claude, tested by me on an actual executable which SignTool.exe refused to sign. It is likely the exact same problem as #124.

Please evaluate and incorporate the fix if appropriate. For me the fix was invaluable, as it allows me to ship executables that are both signed and equipped with exception stack tracing.

---- Claude info and fix follows in attached file ----

we-have-a-problem-happy-parrot.md

[obones]

Could you please paste the content of that file here?
It would make for easier reading.

And ideally, a pull request would make life much easier

[eivindbakkestuen]

JCL bug report + fix: JCLDEBUG section insertion produces a PE that signtool rejects (0x800700C1)

Summary

InsertDebugDataIntoExecutableFile (source/windows/JclDebug.pas) appends a JCLDEBUG
section to a compiled binary to carry the converted MAP / stack-trace data. When the
existing PE header area has no room for one more section header, the routine grows the
header area (shifting all section raw data forward) but never updates
OptionalHeader.SizeOfHeaders, and it also assigns a bogus PointerToRawData to
sections that have no raw data (.bss, .tls).

The Windows loader tolerates the result, so the instrumented EXE runs. But strict PE
validators do not: signtool.exe sign fails with

SignTool Error: SignedCode::Sign returned error: 0x800700C1
   (ERROR_BAD_EXE_FORMAT — "%1 is not a valid Win32 application")

The identical binary without JCL debug data signs without error.

The same latent defect exists in the generic PeInsertSection in
source/windows/JclPeImage.pas.

This report documents the root cause, a fix for both units, and an end-to-end
verification (the fixed tool produces a binary that signtool signs successfully).

Affected: 64-bit and 32-bit PE targets. Reproduced on a Delphi/RAD Studio Win64 binary.

---

How to reproduce

1. Build any Delphi EXE whose PE header block has < 40 free bytes after the section table
   (the common layout — 12 sections in a 0x400 header leaves only 24 free bytes — always
   qualifies; see "Why it is intermittent" below).
2. Insert JCL debug data (IDE Debug Expert, or MakeJclDbg -E <map>, or
   InsertDebugDataIntoExecutableFile).
3. signtool sign /fd SHA256 /sha1 <thumbprint> the.exe → fails with 0x800700C1.

---

Evidence (64-bit PE32+, SectionAlignment=0x1000, FileAlignment=0x200)

Parsed PE headers of the same program built three ways:

Field	clean (no JCL data)	JCL master	JCL fixed
NumberOfSections	12	13 (JCLDEBUG added)	13 (JCLDEBUG added)
Section table ends @	0x3E8	0x410	0x410
SizeOfHeaders	0x400	0x400 (not updated)	0x600 (updated)
SizeOfHeaders >= table end?	yes	NO (0x400 < 0x410)	yes
First section raw offset	0x400 (= SizeOfHeaders)	0x600 (≠ SizeOfHeaders)	0x600 (= SizeOfHeaders)
.bss / .tls PointerToRawData	0 (correct)	0x200 (wrong; RawSize=0)	0 (correct)
SizeOfImage	0x488E000	0x4AB0000 (correct)	0x4AB0000 (correct)
CheckSum	(set)	0x00000000	recomputed
signtool sign	OK	FAIL 0x800700C1	OK (exit 0)

JCLDEBUG payload bytes are unchanged by the fix (only PE-header arithmetic changed), so
runtime trace resolution is unaffected.

---

Root cause

The original header block is 0x400 bytes; with 12 sections the section table ends at
0x3E8, leaving only 24 free bytes — less than one 40-byte IMAGE_SECTION_HEADER. To fit
the 13th (JCLDEBUG) header, the routine (64-bit path) calls CheckHeadersSpace, which:

1. MoveData — shifts all section raw data forward by one FileAlignment (0x200);
   the first section moves 0x400 → 0x600.
2. MovePointerToRawData — adds 0x200 to every section's PointerToRawData.
3. rewrites the section headers.

It does not update OptionalHeader.SizeOfHeaders. So after insertion:

• the section table ends at 0x410 but SizeOfHeaders is still 0x400 → the last section
  header (bytes 0x400–0x410) lies outside the headers region, violating the PE
  rule that the section table resides within SizeOfHeaders; and
• the first section's raw data now begins at 0x600, no longer equal to SizeOfHeaders.

The loader maps a full page for headers regardless, so the image runs.
signtool/wintrust re-derive and validate the layout strictly and reject it
(0x800700C1).

Secondary defect: MovePointerToRawData bumps PointerToRawData for every section,
including .bss/.tls, which have SizeOfRawData = 0 and must keep PointerToRawData = 0.
After the shift they read 0x200. Loaders ignore the pointer when raw size is 0, but it is
incorrect and is produced by the same code path.

Why it is intermittent

The bug only triggers when the original header block has < 40 free bytes, forcing the
MoveData path. The common Delphi layout (12 sections in a 0x400 header → 24 free bytes)
always triggers it; binaries with more header slack skip MoveData and sign fine. This
matches "it works for some builds but not others."

Ruled out (not the cause)

• CheckSum = 0 is not the trigger: 0x800700C1 is a parse-time structural rejection
  that precedes hashing, signtool rewrites the checksum while signing, and unsigned Delphi
  EXEs routinely ship with checksum 0 and sign fine. (We still recompute it — see below —
  as correctness hygiene, not as the fix.)
• Unaligned JCLDEBUG.VirtualSize and unaligned .debug SizeOfRawData are
  normal (VirtualSize is the exact size; the unaligned .debug raw size is
  linker-produced and present in the clean, signable binary too).

---

The fix

1) source/windows/JclDebug.pas — InsertDebugDataIntoExecutableFile (the path the IDE Debug Expert and MakeJclDbg use)

• Update SizeOfHeaders when the header area grows. CheckHeadersSpace is
  generalized to take FileAlignment and a var SizeOfHeaders, and increments
  SizeOfHeaders by one FileAlignment block after MoveData/MovePointerToRawData
  (one block is always sufficient for one 40-byte header). It is then written back as part
  of the existing NtHeaders32/NtHeaders64 write.
• Call CheckHeadersSpace from the 32-bit path too. Previously only the 64-bit path
  called it; the 32-bit path had no room check at all, so a 32-bit EXE lacking header
  slack would get the new header written over the first section's raw data (silent
  corruption). Both paths now share the generalized routine.
• Preserve zero-raw section pointers. MovePointerToRawData now skips sections with
  PointerToRawData = 0.
• Recompute the PE checksum via the existing PeUpdateCheckSum after the exclusive
  stream is closed.

2) source/windows/JclPeImage.pas — generic PeInsertSection32/64

This "generic" copy of the logic (not used by the IDE Debug Expert, but public API)
carried the identical latent defect: it had no header-space check, so inserting a
section into a binary without header slack overwrote the first section's raw data. Added a
shared InsertHeaderSpace helper plus the same room-making + SizeOfHeaders update +
zero-raw-pointer handling in both the 32-bit and 64-bit local functions, and a
PeUpdateCheckSum after SaveToFile. (Pointers into TMemoryStream.Memory are re-fetched
after InsertHeaderSpace, since the buffer may be reallocated.)

Both units compile cleanly (no errors) for Win64 and Win32 with dcc64/dcc32. Note
that taWin32/taWin64 are runtime case arms (not platform IFDEFs), so a single
successful compile type-checks both code paths.

---

Verification (end-to-end)

1. Built MakeJclDbg from the patched sources (dcc64).
2. On an unsigned copy of a 64-bit EXE that reproduces the failure, ran
   MakeJclDbg -E <map> to insert debug data.
3. Re-parsed the PE: SizeOfHeaders 0x400 → 0x600, now >= section-table end and ==
   first-section raw offset; .bss/.tls PointerToRawData = 0; SizeOfImage correct;
   JCLDEBUG payload present with JDBG magic; file size as expected.
4. signtool sign /fd SHA256 /sha1 <thumb> the.exe → Successfully signed (exit 0).
   The same step on the master-built binary fails with 0x800700C1.
5. Confirmed the JCLDEBUG payload is the byte stream JCL always emitted (header
   arithmetic only was changed), so stack-trace resolution is unaffected.

Recommended regression cases for the maintainers:

• A binary with ample header slack (no MoveData/InsertHeaderSpace path) — confirm
  it still signs and still resolves traces (no regression on the common-but-roomy case).
• A 32-bit binary lacking header slack — confirm the newly added 32-bit room-making
  produces a valid, signable image.

---

Known remaining edge case (separate from this bug; not addressed here)

Insertion does not clear the Security/Certificate data directory (index 4). Inserting
JCL data into an already Authenticode-signed binary leaves the directory pointing at a
now-shifted/invalid certificate, so that binary is still unsignable. Normal pipelines build
unsigned → add JCL data → sign, so this does not arise in practice. A robust improvement
would be to zero data-directory entry 4 (offset and size) on insertion when present.

---

Patch

Against source/windows/JclDebug.pas and source/windows/JclPeImage.pas
(unified diff; current function locations: InsertDebugDataIntoExecutableFile at
JclDebug.pas:2609, PeInsertSection at JclPeImage.pas:5276):

diff --git a/jcl/source/windows/JclDebug.pas b/jcl/source/windows/JclDebug.pas
--- a/jcl/source/windows/JclDebug.pas
+++ b/jcl/source/windows/JclDebug.pas
@@ -2631,7 +2631,11 @@ var
     I: Integer;
   begin
     for I := Low(ImageSectionHeaders) to High(ImageSectionHeaders) do
-      ImageSectionHeaders[I].PointerToRawData := ImageSectionHeaders[I].PointerToRawData + AOffset;
+      // Sections with no raw data on disk (e.g. .bss, .tls) have PointerToRawData = 0
+      // and must keep it. Only shift sections that actually have raw data, otherwise
+      // they end up with a bogus non-zero file offset.
+      if ImageSectionHeaders[I].PointerToRawData <> 0 then
+        ImageSectionHeaders[I].PointerToRawData := ImageSectionHeaders[I].PointerToRawData + AOffset;
   end;
 
   procedure FillZeros(AStream: TStream; ACount: Integer);
@@ -2697,13 +2701,19 @@ var
       raise EJclPeImageError.CreateRes(@SWriteError);
   end;
 
-  procedure CheckHeadersSpace(AStream: TStream);
+  procedure CheckHeadersSpace(AStream: TStream; AFileAlignment: DWORD; var ASizeOfHeaders: DWORD);
   begin
     if ImageSectionHeaders[0].PointerToRawData < ImageSectionHeadersPosition +
        (SizeOf(TImageSectionHeader) * (Length(ImageSectionHeaders) + 1)) then
     begin
-      MoveData(AStream, ImageSectionHeaders[0].PointerToRawData, NtHeaders64.OptionalHeader.FileAlignment);
-      MovePointerToRawData(NtHeaders64.OptionalHeader.FileAlignment);
+      MoveData(AStream, ImageSectionHeaders[0].PointerToRawData, AFileAlignment);
+      MovePointerToRawData(AFileAlignment);
+      // The header area was grown by one FileAlignment block to make room for the new
+      // section header. SizeOfHeaders must grow with it; otherwise the section table
+      // extends past SizeOfHeaders and the first section's raw data no longer starts at
+      // SizeOfHeaders. The Windows loader tolerates that, but strict PE validators
+      // (e.g. signtool) reject the image with ERROR_BAD_EXE_FORMAT (0x800700C1).
+      Inc(ASizeOfHeaders, AFileAlignment);
       WriteSectionHeaders(AStream, ImageSectionHeadersPosition);
     end;
   end;
@@ -2759,6 +2769,9 @@ begin
               Exit;
             end;
 
+            // Check if there is enough space for an additional section header
+            CheckHeadersSpace(ImageStream, NtHeaders32.OptionalHeader.FileAlignment, NtHeaders32.OptionalHeader.SizeOfHeaders);
+
             JclDebugSectionPosition := ImageSectionHeadersPosition + (SizeOf(ImageSectionHeaders[0]) * Length(ImageSectionHeaders));
             LastSection := @ImageSectionHeaders[High(ImageSectionHeaders)];
 
@@ -2801,8 +2814,8 @@ begin
               Exit;
             end;
 
-            // Check if there is enough space for additional header
-            CheckHeadersSpace(ImageStream);
+            // Check if there is enough space for an additional section header
+            CheckHeadersSpace(ImageStream, NtHeaders64.OptionalHeader.FileAlignment, NtHeaders64.OptionalHeader.SizeOfHeaders);
 
             JclDebugSectionPosition := ImageSectionHeadersPosition + (SizeOf(ImageSectionHeaders[0]) * Length(ImageSectionHeaders));
             LastSection := @ImageSectionHeaders[High(ImageSectionHeaders)];
@@ -2857,6 +2870,12 @@ begin
   finally
     ImageStream.Free;
   end;
+  // Inserting the section invalidated the original PE checksum. Recompute it now that
+  // the exclusive stream is closed (MapAndLoad needs its own handle). Signing tools
+  // rewrite the checksum themselves, but a correct value is expected by some loaders
+  // and validators.
+  if Result then
+    PeUpdateCheckSum(ExecutableFileName);
 end;
 
 //=== { TJclBinDebugGenerator } ==============================================
diff --git a/jcl/source/windows/JclPeImage.pas b/jcl/source/windows/JclPeImage.pas
--- a/jcl/source/windows/JclPeImage.pas
+++ b/jcl/source/windows/JclPeImage.pas
@@ -5279,11 +5279,43 @@ function PeInsertSection(const FileName: TFileName; SectionStream: TStream; Sect
     if (Value mod Alignment) <> 0 then
       Value := ((Value div Alignment) + 1) * Alignment;
   end;
+  // Insert ACount zero bytes at file offset AOffset in the in-memory image, shifting
+  // everything from AOffset onward forward by ACount. Used to enlarge the header area
+  // so that another section header fits before the first section's raw data.
+  // NOTE: ImageStream.Memory may be reallocated, so callers must re-fetch any pointers
+  // into it afterwards.
+  procedure InsertHeaderSpace(ImageStream: TMemoryStream; AOffset, ACount: DWORD);
+  var
+    OldSize: Int64;
+    CurPos: Int64;
+    ChunkSize: Integer;
+    Buffer: array of Byte;
+  begin
+    OldSize := ImageStream.Size;
+    ImageStream.Size := OldSize + ACount;
+    SetLength(Buffer, 1024 * 1024);
+    CurPos := OldSize;
+    while CurPos > AOffset do
+    begin
+      ChunkSize := Length(Buffer);
+      if CurPos - ChunkSize < AOffset then
+        ChunkSize := Integer(CurPos - AOffset);
+      Dec(CurPos, ChunkSize);
+      ImageStream.Position := CurPos;
+      ImageStream.ReadBuffer(Buffer[0], ChunkSize);
+      ImageStream.Position := CurPos + ACount;
+      ImageStream.WriteBuffer(Buffer[0], ChunkSize);
+    end;
+    // zero the inserted gap (ACount = FileAlignment <= 64K, fits in Buffer)
+    FillChar(Buffer[0], ACount, 0);
+    ImageStream.Position := AOffset;
+    ImageStream.WriteBuffer(Buffer[0], ACount);
+  end;
   function PeInsertSection32(ImageStream: TMemoryStream): Boolean;
   var
     NtHeaders: PImageNtHeaders32;
-    Sections, LastSection, NewSection: PImageSectionHeader;
-    VirtualAlignedSize: DWORD;
+    Sections, LastSection, NewSection, SecPtr: PImageSectionHeader;
+    VirtualAlignedSize, FileAlignment: DWORD;
     I, X, NeedFill: Integer;
     SectionDataSize: Integer;
     UTF8Name: TUTF8String;
@@ -5302,11 +5334,38 @@ function PeInsertSection(const FileName: TFileName; SectionStream: TStream; Sect
         Exit;
       end;
 
+      FileAlignment := NtHeaders^.OptionalHeader.FileAlignment;
+
       LastSection := Sections;
       Inc(LastSection, NtHeaders^.FileHeader.NumberOfSections - 1);
       NewSection := LastSection;
       Inc(NewSection);
 
+      // Make room in the header area when the new section header would not fit before
+      // the first section's raw data. Without this the header is written over the first
+      // section's data; growing SizeOfHeaders in step keeps the image valid for strict
+      // PE validators (e.g. signtool, which otherwise fails with ERROR_BAD_EXE_FORMAT).
+      if TJclAddr(NewSection) - TJclAddr(ImageStream.Memory) + DWORD(SizeOf(TImageSectionHeader)) > Sections^.PointerToRawData then
+      begin
+        InsertHeaderSpace(ImageStream, Sections^.PointerToRawData, FileAlignment);
+        // ImageStream.Memory may have been reallocated -> re-fetch the pointers.
+        NtHeaders := PeMapImgNtHeaders32(ImageStream.Memory);
+        Sections := PeMapImgSections32(NtHeaders);
+        SecPtr := Sections;
+        for I := 0 to NtHeaders^.FileHeader.NumberOfSections - 1 do
+        begin
+          // sections with no raw data on disk keep PointerToRawData = 0
+          if SecPtr^.PointerToRawData <> 0 then
+            Inc(SecPtr^.PointerToRawData, FileAlignment);
+          Inc(SecPtr);
+        end;
+        Inc(NtHeaders^.OptionalHeader.SizeOfHeaders, FileAlignment);
+        LastSection := Sections;
+        Inc(LastSection, NtHeaders^.FileHeader.NumberOfSections - 1);
+        NewSection := LastSection;
+        Inc(NewSection);
+      end;
+
       // Increase the number of sections
       Inc(NtHeaders^.FileHeader.NumberOfSections);
       ResetMemory(NewSection^, SizeOf(TImageSectionHeader));
@@ -5353,8 +5412,8 @@ function PeInsertSection(const FileName: TFileName; SectionStream: TStream; Sect
   function PeInsertSection64(ImageStream: TMemoryStream): Boolean;
   var
     NtHeaders: PImageNtHeaders64;
-    Sections, LastSection, NewSection: PImageSectionHeader;
-    VirtualAlignedSize: DWORD;
+    Sections, LastSection, NewSection, SecPtr: PImageSectionHeader;
+    VirtualAlignedSize, FileAlignment: DWORD;
     I, X, NeedFill: Integer;
     SectionDataSize: Integer;
     UTF8Name: TUTF8String;
@@ -5373,11 +5432,38 @@ function PeInsertSection(const FileName: TFileName; SectionStream: TStream; Sect
         Exit;
       end;
 
+      FileAlignment := NtHeaders^.OptionalHeader.FileAlignment;
+
       LastSection := Sections;
       Inc(LastSection, NtHeaders^.FileHeader.NumberOfSections - 1);
       NewSection := LastSection;
       Inc(NewSection);
 
+      // Make room in the header area when the new section header would not fit before
+      // the first section's raw data. Without this the header is written over the first
+      // section's data; growing SizeOfHeaders in step keeps the image valid for strict
+      // PE validators (e.g. signtool, which otherwise fails with ERROR_BAD_EXE_FORMAT).
+      if TJclAddr(NewSection) - TJclAddr(ImageStream.Memory) + DWORD(SizeOf(TImageSectionHeader)) > Sections^.PointerToRawData then
+      begin
+        InsertHeaderSpace(ImageStream, Sections^.PointerToRawData, FileAlignment);
+        // ImageStream.Memory may have been reallocated -> re-fetch the pointers.
+        NtHeaders := PeMapImgNtHeaders64(ImageStream.Memory);
+        Sections := PeMapImgSections64(NtHeaders);
+        SecPtr := Sections;
+        for I := 0 to NtHeaders^.FileHeader.NumberOfSections - 1 do
+        begin
+          // sections with no raw data on disk keep PointerToRawData = 0
+          if SecPtr^.PointerToRawData <> 0 then
+            Inc(SecPtr^.PointerToRawData, FileAlignment);
+          Inc(SecPtr);
+        end;
+        Inc(NtHeaders^.OptionalHeader.SizeOfHeaders, FileAlignment);
+        LastSection := Sections;
+        Inc(LastSection, NtHeaders^.FileHeader.NumberOfSections - 1);
+        NewSection := LastSection;
+        Inc(NewSection);
+      end;
+
       // Increase the number of sections
       Inc(NtHeaders^.FileHeader.NumberOfSections);
       ResetMemory(NewSection^, SizeOf(TImageSectionHeader));
@@ -5442,7 +5528,11 @@ begin
     end;
 
     if Result then
+    begin
       ImageStream.SaveToFile(FileName);
+      // The inserted section invalidated the original PE checksum; recompute it.
+      PeUpdateCheckSum(FileName);
+    end;
   finally
     ImageStream.Free;
   end;