6
\$\begingroup\$

Context Info

I coded up a program that maps an executable file(.exe .dll mainly) to the program's memory space which allows for easier extraction of the PE header info. I extract the information by simply casting a structure of a certain header to a memory location of the mapped file.

What I'm asking for

Readability and general structure. Some of the naming is horrible and I don't know which is the right alternative. The main function looks like it's all over the place for some reason and feels very hard to read. And anything else of course, I'm sure there are tons of bad things that I'm not aware of.

pe32inf.c

#include <windows.h>
#include <stdio.h>
#include "pe32inf.h"

void Terminate(const char *s);
LPCTSTR DecodeInput(int argc, char *argv[]);
LPVOID Map(LPCTSTR lpFileName);
MZ_DOS SetDOSheader(LPVOID lpFileBase);
COFF SetCOFFheader(LPVOID lpCOFFoffset);
SectionTable SetSectionTable(LPVOID SectionTableOffset, int NumberOfSections);


int main(int argc, char *argv[])
{
    LPCTSTR lpFileName;
    LPVOID lpFileBase;
    MZ_DOS DOSheader; //Naming issues with the headers.
    COFF COFFheader;
    OptionalHeader OPTheader; //I tried to get around the 2 declarations because 1 is going to be unused.
    OptionalHeader64 OPTheader64; //However, I can only think of malloc and then there's inconsistency since this will be a PTR.
    SectionTable SECtable;

    lpFileName = DecodeInput(argc, argv);
    lpFileBase = Map(lpFileName);

    DOSheader = SetDOSheader(lpFileBase); 
    COFFheader = SetCOFFheader(lpFileBase + DOSheader.pe_offset + 0x4); //0x4 To skip PE sig.

    LPVOID lpOptionalHeader = lpFileBase + DOSheader.pe_offset + 0x4 + sizeof(COFF);
    WORD magic = *(WORD*)lpOptionalHeader; 
    if (magic == 0x10b) { //PE32
        OPTheader = *(OptionalHeader*)lpOptionalHeader;
    } else if (magic == 0x20b) { //PE32+
        OPTheader64 = *(OptionalHeader64*)lpOptionalHeader;
    } else {
        Terminate("Unknown PE magic.");
    }

    LPVOID SectionTableOffset = lpOptionalHeader + COFFheader.SizeOfOptionalHeader;
    SECtable = SetSectionTable(SectionTableOffset, COFFheader.NumberOfSections);

    return 0;
}

COFF SetCOFFheader(LPVOID lpCOFFoffset)
{
    COFF COFFheader;
    COFFheader = *(COFF*)lpCOFFoffset;
    return COFFheader;
}

SectionTable SetSectionTable(LPVOID SectionTableOffset, int NumberOfSections)
{
    SectionTable SECtable;
    SECtable.NumberOfSections = NumberOfSections;
    SECtable.sectionHeader = malloc(sizeof(SectionHeader) * SECtable.NumberOfSections);

    for (int i = 0; i < SECtable.NumberOfSections; i++) {
        SECtable.sectionHeader[i] = *(SectionHeader*)(SectionTableOffset + (i * sizeof(SectionHeader)));
    }

    return SECtable;
}

MZ_DOS SetDOSheader(LPVOID lpFileBase)
{
    MZ_DOS DOSheader = *(MZ_DOS*)lpFileBase;
    if (DOSheader.signature != 0x5a4D) { //MZ
        Terminate("MZ signature not found. File is not an executable image.");
    }
    if (*(DWORD*)(DOSheader.pe_offset + lpFileBase) != 0x4550) { //PE\0\0
        Terminate("PE signature not found. File is not an executable image.\n");
    }

    return DOSheader;
}

LPCTSTR DecodeInput(int argc, char *argv[]){
    if (argc < 2) {
        Terminate("No file name specified.");
    }

    LPCTSTR lpFileName = argv[argc - 1];
    if (lpFileName == NULL) {
        Terminate("I mean, it probably won't even be NULL.");
    }

    return lpFileName;
}

LPVOID Map(LPCTSTR lpFileName)
{
    HANDLE hFile;
    HANDLE hFileMapping;
    LPVOID lpFileBase;

    hFile = CreateFile(lpFileName, GENERIC_READ, FILE_SHARE_READ,
                       NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, 0);
    if (hFile == INVALID_HANDLE_VALUE) {
        Terminate("Could not open file.\n");
    }

    hFileMapping = CreateFileMapping(hFile, NULL, PAGE_READONLY,
                                     0, 0, NULL);
    if (hFileMapping == NULL) {
        Terminate("Could not create Mapping.\n");
    }

    lpFileBase = MapViewOfFile(hFileMapping, FILE_MAP_READ, 0, 0, 0);
    if (lpFileBase == NULL) {
        Terminate("Could not Map view of file.\n");
    }

    return lpFileBase;  
}

void Terminate(const char *s) 
{
    //Windows will take care of closing the handles. *gasp*
    printf("%s", s);
    exit(0);
}

pe32inf.h

#include <WinDef.h>

typedef struct //http://www.delorie.com/djgpp/doc/exe/
{
    WORD signature; /* == 0x5a4D */
    WORD bytes_in_last_block;
    WORD blocks_in_file;
    WORD num_relocs;
    WORD header_paragraphs;
    WORD min_extra_paragraphs;
    WORD max_extra_paragraphs;
    WORD ss;
    WORD sp;
    WORD checksum;
    WORD ip;
    WORD cs;
    WORD reloc_table_offset;
    WORD overlay_number;
    BYTE padding[0x1F];
    DWORD pe_offset;
} MZ_DOS;

typedef struct
{
    WORD Machine;
    WORD NumberOfSections;
    DWORD TimeDateStamp;
    DWORD PointerToSymbolTable;
    DWORD NumberOfSymbols;
    WORD SizeOfOptionalHeader;
    WORD Characteristics;
} COFF;

typedef struct
{
    WORD Magic;
    BYTE MajorLinkerVersion;
    BYTE MinorLinkerVersion;
    DWORD SizeOfCode;
    DWORD SizeOfInitializedData;
    DWORD SizeOfUninitializedData;
    DWORD AddressOfEntryPoint;
    DWORD BaseOfCode;
    DWORD BaseOfData;
} StandardFields;

typedef struct
{
    WORD Magic;
    BYTE MajorLinkerVersion;
    BYTE MinorLinkerVersion;
    DWORD SizeOfCode;
    DWORD SizeOfInitializedData;
    DWORD SizeOfUninitializedData;
    DWORD AddressOfEntryPoint;
    DWORD BaseOfCode;
} StandardFields64;

typedef struct
{
    DWORD ImageBase;
    DWORD SectionAlignment;
    DWORD FileAlignment;
    WORD MajorOperatingSystemVersion;
    WORD MinorOperatingSystemVersion;
    WORD MajorImageVersion;
    WORD MinorImageVersion;
    WORD MajorSubsystemVersion;
    WORD MinorSubsystemVersion;
    DWORD Win64VersionValue;
    DWORD SizeOfImage;
    DWORD SizeOfHeaders;
    DWORD CheckSum;
    WORD Subsystem;
    WORD DllCharacteristics;
    DWORD SizeOfStackReserve;
    DWORD SizeOfStackCommit;
    DWORD SizeOfHeapReserve;
    DWORD SizeOfHeapCommit;
    DWORD LoaderFlags;
    DWORD NumberOfRvaAndSizes;
} WindowsFields;

typedef struct
{
    DWORDLONG ImageBase;
    DWORD SectionAlignment;
    DWORD FileAlignment;
    WORD MajorOperatingSystemVersion;
    WORD MinorOperatingSystemVersion;
    WORD MajorImageVersion;
    WORD MinorImageVersion;
    WORD MajorSubsystemVersion;
    WORD MinorSubsystemVersion;
    DWORD Win64VersionValue;
    DWORD SizeOfImage;
    DWORD SizeOfHeaders;
    DWORD CheckSum;
    WORD Subsystem;
    WORD DllCharacteristics;
    DWORDLONG SizeOfStackReserve;
    DWORDLONG SizeOfStackCommit;
    DWORDLONG SizeOfHeapReserve;
    DWORDLONG SizeOfHeapCommit;
    DWORD LoaderFlags;
    DWORD NumberOfRvaAndSizes;
} WindowsFields64;

typedef struct
{
    IMAGE_DATA_DIRECTORY ExportTable;
    IMAGE_DATA_DIRECTORY ImportTable;
    IMAGE_DATA_DIRECTORY ResourceTable;
    IMAGE_DATA_DIRECTORY ExceptionTable;
    IMAGE_DATA_DIRECTORY CertificateTable;
    IMAGE_DATA_DIRECTORY BaseRelocationTable;
    IMAGE_DATA_DIRECTORY Debug;
    IMAGE_DATA_DIRECTORY Architecture;
    IMAGE_DATA_DIRECTORY GlobalPTR;
    IMAGE_DATA_DIRECTORY TLStable;
    IMAGE_DATA_DIRECTORY LoadConfigTable;
    IMAGE_DATA_DIRECTORY BoundImport;
    IMAGE_DATA_DIRECTORY IAT;
    IMAGE_DATA_DIRECTORY DelayImportDescriptor;
    IMAGE_DATA_DIRECTORY CLRruntimeHeader;
    IMAGE_DATA_DIRECTORY Reserved;
} DataDirectories;

typedef struct
{
    StandardFields standardFields;
    WindowsFields windowsFields;
    DataDirectories dataDirectories;
} OptionalHeader;

typedef struct
{
    StandardFields64 standardFields; //Naming issue?
    WindowsFields64 windowsFields;
    DataDirectories dataDirectories;
} OptionalHeader64;

typedef struct
{
    DWORDLONG Name;
    DWORD VirtualSize;
    DWORD VirtualAddress;
    DWORD SizeOfRawData;
    DWORD PointerToRawData;
    DWORD PointerToRelocations;
    DWORD PointerToLinenumbers;
    WORD NumberOfRelocations;
    WORD NumberOfLinenumbers;
    DWORD Characteristics;
} SectionHeader;

typedef struct
{
    int NumberOfSections;
    SectionHeader *sectionHeader;
} SectionTable;
\$\endgroup\$
3
\$\begingroup\$
  • pe32.h

    Since you call Windows APIs, it is reasonable to expect that the program targets Windows platform. You really should take advantage of ImageHlp APIs and structures, and in any case not rely on the third party 5-year-old documents.

  • Copying structures

    seems unreasonable. Data are already in memory, so setting up a pointer should suffice. For example:

        COFF * COFFheader;
        ....
        COFFheader * SetCoffHeader(lpFileBase, offset) {
            return (COFFheader *) (lpFileBase + offset);
        }
    

    Same applies to other structures.

  • Magic number

    I mean, 0x4, which according to a comment is a size of PE signature. I recommend to explicitly define a struct PE_signature, take its sizeof and remove the comment.

  • void pointer arithmetic

    LPVOID is defined as typedef void *LPVOID;. I am surprised that lpFileBase + smth compiles at all.

  • Command line parsing

    I don't think that lpFileName == NULL is ever possible.

\$\endgroup\$
  • \$\begingroup\$ Void pointer is allowed in the GNU compiler. stackoverflow.com/questions/13113301/… I'll change it anyway to proper C. \$\endgroup\$ – Majiick Sep 30 '15 at 14:10
  • \$\begingroup\$ void pointer arithmetic* \$\endgroup\$ – Majiick Sep 30 '15 at 14:20
  • \$\begingroup\$ I meant that grammar fix for my own comment... haha, anyway. \$\endgroup\$ – Majiick Sep 30 '15 at 19:27
-1
\$\begingroup\$

- naming conventions

Since you are in Windows world use camelCase and Hungarian notation for variables and preferably for fields too (and not underscore_case or PascalCase) - keep camelCase + Hungarian notation as consistent naming convention. For example DOSheader must preferably be mdDOSHeader and similarly bytes_in_last_block must be wBytesInLastBlock.

- error reporting

Similar to DWORD dwErrorMessageID = GetLastError(); in your Terminate function

- use #defines

To improve readability, try to avoid constants directly such as 0x10b

\$\endgroup\$
  • \$\begingroup\$ Hungarian notation is frowned upon. Casing is usually language dependent, not OS dependent. \$\endgroup\$ – Mast Sep 29 '15 at 19:30
  • \$\begingroup\$ stackoverflow.com/questions/111933/… - interesting read \$\endgroup\$ – Abhi Sep 29 '15 at 19:33
  • \$\begingroup\$ I'm well aware of the history of the notation. That doesn't make it's usage right. \$\endgroup\$ – Mast Sep 29 '15 at 19:34
  • \$\begingroup\$ It is a program using Win32 Written in 'C' - hence I wrote 'windows world' \$\endgroup\$ – Abhi Sep 29 '15 at 19:34
  • 2
    \$\begingroup\$ Please, a discussion of Hungarian without distinguishing between Apps Hungarian and Systems Hungarian is pointless. \$\endgroup\$ – Ben Voigt Sep 29 '15 at 19:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.