PE Executable gets corrupted when adding a new section to it

Question

I am trying to add a new section to a portable executable, where I need to write some data, in this case an entire file.

The second file is able to parse itself and read the data from the section I created inside of it, but for some reason, it gets corrupted when I modify it using the code below.

The Imgur links are below: I'm sorry for the bad formatting :(

This messagebox should appear.

But I get this error message instead: "This app cannot run on your pc."

The new section gets added properly inside the PE:

The data inside the new section:

I can't really tell what is wrong here.

#include <iostream>
#include <Windows.h>
#include <ShlObj.h>
#include <Shlwapi.h>
 
#pragma comment(lib, "Shell32.lib")
#pragma comment(lib, "Shlwapi.lib")
 
DWORD align(DWORD size, DWORD align, DWORD addr) {
    if (!(size % align))
        return addr + size;
    return addr + (size / align + 1) * align;
}
 
int main(int argc, char* argv[])
{
    if (argc < 3)
    {
        std::cout << "Argomenti insufficienti.\n";
        return 0;
    }
 
    char PathToSave[MAX_PATH];
 
    if (SUCCEEDED(SHGetFolderPathA(NULL, CSIDL_COMMON_DOCUMENTS, NULL, 0, PathToSave)))
    {
        PathAppendA(PathToSave, "Bind.exe");
    }
 
    HANDLE fOutput = CreateFileA(PathToSave, GENERIC_WRITE, FILE_SHARE_WRITE, NULL, CREATE_ALWAYS, NULL, NULL);         // Unused
 
    HANDLE FirstFile = CreateFileA(argv[5], GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, NULL, NULL);            // File to read data from
    if (FirstFile == INVALID_HANDLE_VALUE)
    {
        std::cout << "Impossibile aprire il file passato come primo argomento.\n";
        return 0;
    }
 
    HANDLE SecFile = CreateFileA(argv[5], GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, NULL, NULL);               // File to write the read data in
    if (SecFile == INVALID_HANDLE_VALUE)
    {
        std::cout << "Impossibile aprire il file passato come secondo argomento.\n";
        return 0;
    }
 
    DWORD FirstFS = GetFileSize(FirstFile, 0);              // First file dimension
    DWORD SecondFS = GetFileSize(SecFile, 0);               // Second file dimension
 
    BYTE* FirstFB = (BYTE*)VirtualAlloc(NULL, FirstFS, MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE);           // Allocates memory for the first file
    BYTE* SecondFB = (BYTE*)VirtualAlloc(NULL, SecondFS, MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE);         // Allocates memory for the second file
 
    DWORD BytesRead = 0;
    DWORD BytesWritten = 0;
 
    if (bool Read = ReadFile(FirstFile, FirstFB, FirstFS, &BytesRead, NULL) == FALSE)                   // Reads the first file
    {
        std::cout << "Impossibile leggere primo file.\n";
        return 0;
    }
    else
    {
        std::cout << "Letti " << BytesRead << " dal primo file.\n";
        BytesRead = 0;
    }
 
    if (bool Read = ReadFile(SecFile, SecondFB, SecondFS, &BytesRead, NULL) == FALSE)                       // Reads the second file
    {
        std::cout << "Impossibile leggere secondo file.\n";
        return 0;
    }
    else
    {
        std::cout << "Letti " << BytesRead << " bytes dal secondo file.\n";
        BytesRead = 0;
    }
 
    /*
    * 
    * The code is problematic beyond this point!
    * 
    * SecondFB = Pointer to the second file's data buffer that needs to be modified by adding the new section.
    * FirstFB = Pointer to the first file's data buffer that will be written inside the ".sdata" section.
    * Both of them have been loaded in memory using VirtualAlloc.
    * 
    * Ask me anything for further info and many, many thanks :D
    
    */
 
    // Here I add a new section to the second file.
 
    PIMAGE_DOS_HEADER sIDH = (IMAGE_DOS_HEADER*)SecondFB;                                              
    PIMAGE_NT_HEADERS sINH = (IMAGE_NT_HEADERS*)(SecondFB + sIDH->e_lfanew);
    PIMAGE_FILE_HEADER sIFH = (PIMAGE_FILE_HEADER)(SecondFB + sIDH->e_lfanew + sizeof(DWORD));
    PIMAGE_OPTIONAL_HEADER sIOH = (PIMAGE_OPTIONAL_HEADER)(SecondFB + sIDH->e_lfanew + sizeof(DWORD) + sizeof(IMAGE_FILE_HEADER));
    PIMAGE_SECTION_HEADER sISH = (PIMAGE_SECTION_HEADER)(SecondFB + sIDH->e_lfanew + sizeof(IMAGE_NT_HEADERS));
 
    // Here I name the new section inside the file
    ZeroMemory(sISH, sizeof(IMAGE_SECTION_HEADER));
    CopyMemory(sISH[sIFH->NumberOfSections].Name, ".scode", 8);
 
    /*
        0xE00000E0 = IMAGE_SCN_MEM_WRITE |
                     IMAGE_SCN_CNT_CODE  |
                     IMAGE_SCN_CNT_UNINITIALIZED_DATA  |
                     IMAGE_SCN_MEM_EXECUTE |
                     IMAGE_SCN_CNT_INITIALIZED_DATA |
                     IMAGE_SCN_MEM_READ
 
   
    */
 
    // Here all the required information gets filled in
    sISH[sIFH->NumberOfSections].VirtualAddress = align(sISH[sIFH->NumberOfSections - 1].Misc.VirtualSize, sIOH->SectionAlignment, sISH[sIFH->NumberOfSections - 1].VirtualAddress);
    sISH[sIFH->NumberOfSections].SizeOfRawData = align(FirstFS, sIOH->SectionAlignment, 0);
    sISH[sIFH->NumberOfSections].Misc.VirtualSize = align(FirstFS, sIOH->SectionAlignment, 0);
    sISH[sIFH->NumberOfSections].PointerToRawData = align(sISH[sIFH->NumberOfSections - 1].SizeOfRawData, sIOH->FileAlignment, sISH[sIFH->NumberOfSections - 1].PointerToRawData);
    sISH[sIFH->NumberOfSections].Characteristics = 0xE00000E0;
 
    // Here the changes are written to the second file
    SetFilePointer(SecFile, sISH[sIFH->NumberOfSections].PointerToRawData + sISH[sIFH->NumberOfSections].SizeOfRawData, NULL, FILE_BEGIN);
    SetEndOfFile(SecFile);
 
    sIOH->SizeOfImage = sISH[sIFH->NumberOfSections].VirtualAddress + sISH[sIFH->NumberOfSections].Misc.VirtualSize;
    sIFH->NumberOfSections += 1;
    SetFilePointer(SecFile, 0, NULL, FILE_BEGIN);
 
    BytesWritten = 0;
 
    bool W = WriteFile(SecFile, SecondFB, SecondFS, &BytesWritten, NULL);
 
    if (W == FALSE)
    {
        std::cout << "Impossibile aggiungere sezione alla stub.\n";
        return 0;
    }
    else
    {
        std::cout << "Scritti " << BytesWritten << " bytes nella stub. (Aggiunta nuova sezione.)\n";
        BytesWritten = 0;
    }
 
    // Here I write the data inside the new section
    SetFilePointer(SecFile, sISH[sIFH->NumberOfSections - 1].PointerToRawData, 0, FILE_BEGIN);
    if (bool Write = WriteFile(SecFile, FirstFB, FirstFS, &BytesWritten, NULL) == FALSE)
    {
        std::cout << "Impossibile aggiungere sezione alla stub.\n";
    }
    else
    {
        std::cout << "Scritti " << BytesWritten << " bytes nella stub.\n";
        BytesWritten = 0;
    }
 
// Here I close all the handles
 
    VirtualFree(FirstFB, FirstFS, MEM_RELEASE);
    CloseHandle(FirstFile);
 
    VirtualFree(SecondFB, SecondFS, MEM_RELEASE);
    CloseHandle(SecFile);
 
    std::cout << "Binding completato.\n";
 
    return 0;
}

Answer 1

The problem is in ZeroMemory(sISH, sizeof(IMAGE_SECTION_HEADER));

You removed the memory of the first section, which caused problems with the section structure of the exe. This is why the exe cannot run.

Solution:

=> ZeroMemory(&sISH[sIFH->NumberOfSections], sizeof(IMAGE_SECTION_HEADER)); //Clear the memory behind the last section

Don't forget to add FILE_SHARE_WRITE for the first CreateFileA, otherwise it will cause the second CreateFileA to fail to write.

=> HANDLE FirstFile = CreateFileA(argv[5], GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, NULL, NULL); // File to read data from