Intro

A few days ago I was looking for software to remove a digital code signature from an executable file in Windows. I was doing some binary patching on it, which would break its signature. Thus I wanted to remove it.

But to my surprise, I couldn't find such software. So I decided to write my own and also to record myself doing it. My goal was to share the process of coding a production-style Windows application with Visual Studio, including the correct error handling, developer safety tips, debugging and further bug fixes, improvements, and even coding a separate test fuzzing app.

So if you're not into reading blog posts, please watch the videos below. Or download the final binary or the source code.

Table Of Contents

For the ease of navigation, here's the table of contents:

1. Basic Scaffolding, Safety Tips, Error Handling:
   • Debugging Configuration Macros
   • Commenting Your Work
   • Picking Readable Function And Variable Names
   • Error Handling
   • Removing Dangling Pointers
   • Video Overview - Part 1
2. Coding Digital Signature Removal:
   • Digital Signature Specific
   • Video Overview - Part 2
3. Finishing Touches, Handling Command Line:
   • Video Overview - Part 3
4. Bug Fixes, Refactoring & Improvements:
   • Video Overview - Part 4
5. More Bug Fixes & Coding Test Fuzzer App:
   • Adjusting SigRemover Source Code For Fuzzing
   • Test Fuzzer App
   • Video Overview - Part 5
6. Downloads

Basic Scaffolding, Safety Tips, Error Handling

Like any other project, I needed to understand the basic concept of what I was trying to achieve. In my case I was removing a digital signature from a PE file, and thus I needed to understand its structure. Luckily, it is very well documented by Microsoft.

So the goal of my app, that I decided to call SigRemover, was to do the following:

• Parse an input file as if it was a PE binary. (Do all safety checks, assuming "malicious input" from the user.)
• Locate the IMAGE_DIRECTORY_ENTRY_SECURITY directory it its header and make sure it's not empty and valid.
• Then null the entries in IMAGE_DIRECTORY_ENTRY_SECURITY.
• Cut off the binary file where the signature was previously attached.
• Calculate the new checksum on the adjusted PE binary.
• Save the result into a new PE file.

But before that, I needed to code some basic scaffolding for the application.

Debugging Configuration Macros

First I needed to add some helper debugging macros that would assist me with doing unit tests while running a debugging configuration build.

I personally prefer using the assert macro in my code, that is evaluated for false statements only in debugging configuration builds. Here's an example:

assert(divisor != 0);
int quotient = dividend / divisor;

If divisor is 0, this will create an assertion in a debugging build, while that assert() statement will be completely omitted in the release build.

But sometimes, you may prefer to evaluate the statement and also to keep it in the release build. For that I wrote my own verify macro:

#ifdef _DEBUG
#define verify(f) assert(f)
#else
#define verify(f) (f)
#endif

It can be used as such:

verify(CloseHandle(hFile));

Note that the CloseHandle function call will be evaluated with assertion for returning false in the debugging configuration build, while in release build, it will be just a pure CloseHandle function call without any checks.

This type of coding usually helps to root out algorithmic bugs during the initial development and testing phase.

Commenting Your Work

Providing easy-to-follow comments for class members and function calls is important for creating a readable source code. I tried to show this in my demo project.

For instance, the error handling function getFormattedErrorMsg has my own style of function comments that briefly describe the input parameters and a return value:

const WCHAR* CSigRem::getFormattedErrorMsg(int nOSError, WCHAR* pBuffer, size_t szchBuffer)
{
	//'pBuffer' = buffer to fill in with error description
	//'szchBuffer' = size of 'pBuffer' in WCHARs
	//RETURN:
	//		= Pointer to 'pBuffer' (always NULL-terminated)

	//...
}

Also providing description of individual actions in the code helps with its readability:

//Safety null
pBuffer[szchBuffer - 1] = 0;

Keep in mind that the source code comments are not included in the resulting binary, they stay only in the source code (text) files.

In this case the person reading the source code won't have to guess what that particular line of code is doing.

Picking Readable Function And Variable Names

Naming your functions and variables with what their actual purpose is becomes another way to make your source code readable for other developers.

For instance, instead of returning predefined integer constants as a result of the operation, I defined the following global enum with the enumeration of all possible error codes that the app can generate:

enum EXIT_CODES {
	XC_Success = 0,
	XC_BinaryHasNoSignature = 1,

	XC_GEN_FAILURE = -1,
	XC_FailedToOpen = -2,
	XC_Not_PE_File = -3,
	XC_BadSignature = -4,
	XC_FailedChecksum = -5,
	XC_FailedFileWrite = -6,
};

Having done that will allow us to navigate through our further task of coding a separate test fuzzer app with ease.

Error Handling

Doing a proper error handling in a production application is paramount. I can't stress it enough. How often do you see an app that fails with a silly error message that outputs some error code that does nothing but to confuse the user? So don't be that app developer!

You will see in my video demonstration that I spent a considerable amount of time designing and coding proper error handling functions. At first this seems like a waste of time. But it isn't. This is a proper way to treat your users to errors. The main goal here is to make errors understandable for your general users and also to provide some technical details to help developers, and support personnel for your app (if you have any) to deal with exceptional situations.

For the purpose of this app I named my general error handling function as ReportOSError. Its main purpose is two-fold:

1. To report OS error codes as localized error messages.
2. To provide additional, app-specific text message for the error using a variadic list of parameters.

void CSigRem::ReportOSError(int nOSError, LPCTSTR pStrFmt, ...)
{
	//Pick the right format for the error code
	WCHAR buffErrCode[32];
	buffErrCode[0] = 0;
	if ((UINT)nOSError & 0xC0000000)
	{
		//Hex
		verify(SUCCEEDED(::StringCchPrintf(buffErrCode, _countof(buffErrCode), L"0x%X", nOSError)));
	}
	else
	{
		//Unsigned int
		verify(SUCCEEDED(::StringCchPrintf(buffErrCode, _countof(buffErrCode), L"%u", nOSError)));
	}

	buffErrCode[_countof(buffErrCode) - 1] = 0;

	//Format error message
	WCHAR buffError[1024];
	buffError[0] = 0;
	getFormattedErrorMsg(nOSError, buffError, _countof(buffError));

	//Do we have a user message?
	if (pStrFmt &&
		pStrFmt[0])
	{
		//We have a user message
		va_list argList;
		va_start(argList, pStrFmt);

		//Get length
		int nLnBuff = _vscwprintf(pStrFmt, argList);

		//Reserve a buffer
		WCHAR* pBuff = new (std::nothrow) WCHAR[nLnBuff + 1];
		if (pBuff)
		{
			//Do formatting
			vswprintf_s(pBuff, nLnBuff + 1, pStrFmt, argList);
			pBuff[nLnBuff] = 0;

			wprintf(L"%s\n"
				L"ERROR: (%s) %s\n"
				, 
				pBuff, 
				buffErrCode, 
				buffError);

			//Free mem
			delete[] pBuff;
			pBuff = NULL;
		}
		else
			assert(false);

		va_end(argList);
	}
	else
	{
		//No user message
		wprintf(L"ERROR: (%s) %s\n", buffErrCode, buffError);
	}

	//Restore last error
	::SetLastError(nOSError);
}

As you can see from the code above, the function not only performs a proper formatting of the error code itself (by selecting either a hexadecimal or a decimal output) but it also retrieves a localized error description for it in my other helper function that I called getFormattedErrorMsg:

const WCHAR* CSigRem::getFormattedErrorMsg(int nOSError, WCHAR* pBuffer, size_t szchBuffer)
{
	//'pBuffer' = buffer to fill in with error description
	//'szchBuffer' = size of 'pBuffer' in WCHARs
	//RETURN:
	//		= Pointer to 'pBuffer' (always NULL-terminated)
	int nPrev_OSError = ::GetLastError();

	if (szchBuffer)
	{
		if (nOSError)
		{
			LPVOID lpMsgBuf = NULL;
			DWORD dwRes;

			pBuffer[0] = 0;

			dwRes = FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
				NULL,
				nOSError,
				MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
				(LPTSTR)&lpMsgBuf, 0, NULL);

			if (lpMsgBuf)
			{
				verify(SUCCEEDED(::StringCchCopy(pBuffer, szchBuffer, (LPCTSTR)lpMsgBuf)));
				::LocalFree(lpMsgBuf);
				lpMsgBuf = NULL;
			}

			//Safety null
			pBuffer[szchBuffer - 1] = 0;

			//Remove all \n and \r chars
			for (WCHAR* pS = pBuffer; ; pS++)
			{
				WCHAR z = *pS;
				if (!z)
					break;

				if (z == L'\n' || z == L'\r')
					*pS = L' ';
			}
		}
		else
		{
			//No errors
			pBuffer[0] = 0;
		}
	}
	else
		assert(false);

	::SetLastError(nPrev_OSError);
	return pBuffer;
}	

Note that the getFormattedErrorMsg function employs a safety mechanism to protect against buffer overflows by requiring a caller to specify the size of the output buffer in its szchBuffer parameter. It then uses it in the call to the StringCchCopy Windows function that fills in the buffer, that is a safe alternative to the old strcpy C function.

In despite of an obvious ubiquity of the old-style unsafe C functions like strcpy, sprintf and others, it is very important not to use them in any new code that you write, and to attempt to replace them in your existing code.

The main concern with these unsafe functions is that they may cause a buffer overflow vulnerability in your app if their input comes directly from the user.

Then after we made our error handing function we need to remember to use it in every place in our code where some other function can fail.

And don't just gloss over possible error results from your system function calls, as "Oh, it will never happen." Believe me, it will. Windows is installed on a myriad of different hardware configurations, and if some API can return an error, on some unconventional system it will. Mark my words for it.

A good example of use of our error handling function is during the opening of a PE file:

//Open file for reading
HANDLE hFile = ::CreateFile(pStrFilePath, GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
if (hFile != INVALID_HANDLE_VALUE)
{
	LARGE_INTEGER liFileSz = {};
	if (::GetFileSizeEx(hFile, &liFileSz))
	{
		//Make sure the file is not too large
		if ((ULONGLONG)liFileSz.QuadPart < INT_MAX)
		{
			//.... more code ....
			
		}
		else
			ReportOSError(8312, L"File is too large: %s", pStrFilePath);
	}
	else
		ReportOSError(::GetLastError(), L"Failed to get file size: %s", pStrFilePath);

	//Close handle
	verify(::CloseHandle(hFile));
}
else
	ReportOSError(::GetLastError(), L"Failed to open binary file: %s", pStrFilePath);

As you can see, we not only provide simple error descriptions to the user, we also specify appropriate OS error codes. In two obvious cases, we get them from the API call itself by employing GetLastError. But in the third case, we can't use GetLastError as there's no system API that could fill it in for us. In that case we need to find an appropriate error code ourselves. I'm using WinAPI Search app for that, as I've shown in the video. I chose error code 8312 for that situation, that stands for "The specified object is too large".

Removing Dangling Pointers

Additionally in the code above, it was also wise to NULL-out freed pointers in your code. This applies to most of the memory allocated from the heap, like the new or malloc functions, or to any system API that does it implicitly for you.

Here's an example. Note that after FormatMessage succeeds, and returns a pointer to an allocated array in lpMsgBuf, we use it in the StringCchCopy function and then free allocated memory with a call to LocalFree. After LocalFree function returns, the lpMsgBuf pointer technically will become invalid and should not be used further in the code. But that is easier said than done, and this behavior becomes a subject to a nasty class of bugs, called "Use after free", or a "Dangling Pointer" bug. Thus, it is somewhat prudent to NULL out our pointer right after the LocalFree call, as such:

if (lpMsgBuf)
{
	verify(SUCCEEDED(::StringCchCopy(pBuffer, szchBuffer, (LPCTSTR)lpMsgBuf)));
	::LocalFree(lpMsgBuf);
	lpMsgBuf = NULL;
}

This way if we attempt to use it again after de-allocation, our app with immediately crash and we will see our mistake. This of course uses up some CPU cycles and uses up a little bit of space in the code itself, but in my view, such price is very much worth it.

Video Overview - Part 1

Lastly, here's a full recording of what I described above. Make sure to play it full screen:

Coding Digital Signature Removal

On the next phase of this demo project I went into coding the actual function that parses PE file and removes the digital signature from it. You wouldn't believe me, but I named it RemoveDigitalSignature.

Most of the code in it is quite straightforward. It first opens a binary file and reads it into memory.

I need to confess that I went with a simpler approach of allocating memory and reading the entire PE file into it. This is not the ideal way of doing it, as such approach may fail if our PE file is too large. In that case, a more appropriate way would have been to read it in chunks. But that would unnecessarily complicate this simple test app, and thus I chose against it.

My function then employs another internal function, called process_PE_File, that parses the PE header from memory, and when it returns success, RemoveDigitalSignature saves the result in a new PE file. All these steps are quite mundane, so I will not be explaining them here.

What deserves some attention though is the process_PE_File function itself. Here's a short snippet of it:

EXIT_CODES CSigRem::process_PE_File(BYTE* pBaseAddr, ULONG szcbMem, ULONG& uicbNewFileSz, int& nOSErr)
{
	//'pBaseAddr' = pointer to the beginning of the PE file (it should not be mapped!)
	//'szcbMem' = size of 'pBaseAddr' in BYTEs
	//'uicbNewFileSz' = receives new file size in BYTEs after signature has been removed (valid only if result is XC_Success)
	//'nOSErr' = receives OS error code, if any
	BYTE* pEndAddr = pBaseAddr + szcbMem;

	if ((LONG)szcbMem < sizeof(IMAGE_DOS_HEADER))
	{
		//Error
		nOSErr = ERROR_BAD_EXE_FORMAT;
		return XC_Not_PE_File;
	}

	//Define DOS header
	IMAGE_DOS_HEADER* pDosHdr = (IMAGE_DOS_HEADER*)pBaseAddr;
	size_t szcbNtHdr = (ULONG)pDosHdr->e_lfanew + sizeof(IMAGE_NT_HEADERS64);		//Assume the worst case (or 64-bit)

	if (szcbNtHdr > szcbMem)
	{
		//Error
		nOSErr = ERROR_BAD_EXE_FORMAT;
		return XC_Not_PE_File;
	}

	//Define NT headers
	IMAGE_NT_HEADERS* pNtHdr = (IMAGE_NT_HEADERS*)(pBaseAddr + (ULONG)pDosHdr->e_lfanew);
	if (pNtHdr->Signature != IMAGE_NT_SIGNATURE)
	{
#ifndef FUZZING_BUILD
		//Error
		nOSErr = ERROR_BAD_EXE_FORMAT;
		return XC_Not_PE_File;
#endif
	}

	//Get to sections
	IMAGE_SECTION_HEADER* pSections = (IMAGE_SECTION_HEADER*)(IMAGE_FIRST_SECTION(pNtHdr));
	if (CHECK_PTR_4_OVERRUN(pSections, pEndAddr))
	{
		//Error
		nOSErr = ERROR_BAD_EXE_FORMAT;
		return XC_Not_PE_File;
	}


	//Determine bitness and get some other info from the headers
	IMAGE_NT_HEADERS32* pNtHdr32 = NULL;
	IMAGE_NT_HEADERS64* pNtHdr64 = NULL;
	IMAGE_DATA_DIRECTORY* pDataDirectories = NULL;
	DWORD* pdwChecksum = NULL;

	switch (pNtHdr->OptionalHeader.Magic)
	{
		case IMAGE_NT_OPTIONAL_HDR32_MAGIC:
		{
			//32-bit
			pNtHdr32 = (IMAGE_NT_HEADERS32*)pNtHdr;
			IMAGE_OPTIONAL_HEADER32* pIOH32 = &pNtHdr32->OptionalHeader;
			if (CHECK_PTR_4_OVERRUN(pIOH32, pEndAddr))
			{
				//Error
				nOSErr = ERROR_BAD_EXE_FORMAT;
				return XC_Not_PE_File;
			}

			pDataDirectories = pIOH32->DataDirectory;
			pdwChecksum = &pIOH32->CheckSum;
		}
		break;

		case IMAGE_NT_OPTIONAL_HDR64_MAGIC:
		{
			//64-bit
			pNtHdr64 = (IMAGE_NT_HEADERS64*)pNtHdr;
			IMAGE_OPTIONAL_HEADER64* pIOH64 = &pNtHdr64->OptionalHeader;
			if (CHECK_PTR_4_OVERRUN(pIOH64, pEndAddr))
			{
				//Error
				nOSErr = ERROR_BAD_EXE_FORMAT;
				return XC_Not_PE_File;
			}

			pDataDirectories = pIOH64->DataDirectory;
			pdwChecksum = &pIOH64->CheckSum;
		}
		break;

		default:
		{
			//Error
			nOSErr = ERROR_BAD_EXE_FORMAT;
			return XC_Not_PE_File;
		}
	}


	assert(pDataDirectories);
	assert(pdwChecksum);


	//We need to examine IMAGE_DIRECTORY_ENTRY_SECURITY
	IMAGE_DATA_DIRECTORY* pID = &pDataDirectories[IMAGE_DIRECTORY_ENTRY_SECURITY];
	if (CHECK_PTR_4_OVERRUN(pID, pEndAddr))
	{
		//Error
		nOSErr = ERROR_BAD_EXE_FORMAT;
		return XC_Not_PE_File;
	}


	//See if we have any signature?
	if (!pID->Size &&
		!pID->VirtualAddress)
	{
		//No signature
		return XC_BinaryHasNoSignature;
	}


	//We will assume that the signature is always at the end of the binary file
	if (pID->VirtualAddress + pID->Size != szcbMem)
	{
		//Signature is not at the end of file
		nOSErr = 1466;
		return XC_BadSignature;
	}



	//Now start modifying the binary
	////////////////////////////////////////////////
	
	//....
}

There are several things to note here:

• Notice that I am providing ample comments for the entry parameters for the function itself, and also for every relevant chunk of code within it. This is needed only to help other developers that may work with this code to quickly see what I am doing there.
• Another important aspect of writing this function is to understand that its input may be coming from an untrusted source, and thus it has to be properly vetted before it can be used. What I mean by that is that each offset inside the PE header structure must be checked for overruns. I'm achieving this by checking the PE header offsets in every code block against the size of our memory buffer before it is used to access memory in it.

  For instance, this initial check ensures that our provided PE file is at least larger than the size of the IMAGE_DOS_HEADER before we attempt to read from it:

  C++[Copy]

  if ((LONG)szcbMem < sizeof(IMAGE_DOS_HEADER))
  {
  	//Error
  	nOSErr = ERROR_BAD_EXE_FORMAT;
  	return XC_Not_PE_File;
  }

• I also coded a separate macro for verification of PE header pointers before I attempt to use them. This is also important to do in any kind of parser, or deserializer:
  C++[Copy]

  #define CHECK_PTR_4_OVERRUN(p_s, end) 	((BYTE*)(p_s) >= (end) || (BYTE*)(p_s) + sizeof(*(p_s)) >= (end))

  I can then use this macro for a check like so:

  C++[Copy]

  //Get to sections
  IMAGE_SECTION_HEADER* pSections = (IMAGE_SECTION_HEADER*)(IMAGE_FIRST_SECTION(pNtHdr));
  if (CHECK_PTR_4_OVERRUN(pSections, pEndAddr))
  {
  	//Error
  	nOSErr = ERROR_BAD_EXE_FORMAT;
  	return XC_Not_PE_File;
  }

  Where I previously set pEndAddr as such:

  C++[Copy]

  BYTE* pEndAddr = pBaseAddr + szcbMem;

• I also use asserts to verify my coding logic on the development stage. For instance, I use these two checks in the part of the function where these two pointers have to be set to proper values:
  C++[Copy]

  assert(pDataDirectories);
  assert(pdwChecksum);

  If any of them is still NULL, this means that my previous logic (say, in a preceding switch) did not account for them, or set them correctly. This is just a small safety mechanism to prevent what is known as "unitialized pointers".

• It is also important to initialize your variables and especially pointers (to NULLs) before you begin using them. This wastes a little bit of CPU cycles, but it also creates a much safer code that is easy to debug and review.
  C++[Copy]

  IMAGE_DATA_DIRECTORY* pDataDirectories = NULL;
  DWORD* pdwChecksum = NULL;

Lastly, note how we check if the PE file has a digital signature. This part is very easy to do. We just need to see if the IMAGE_DIRECTORY_ENTRY_SECURITY is empty:

//See if we have any signature?
if (!pID->Size &&
	!pID->VirtualAddress)
{
	//No signature
	return XC_BinaryHasNoSignature;
}

If it is empty, then we return an appropriate warning that signifies that the PE file has no signature. In this case we don't have to do anything else. (This is technically not an error though.)

Digital Signature Specific

For the sake of this app, I made an assumption (following my experience with signed binary files in Windows) that the digital signature will be always placed at the end of the PE file. I check it in this line of code:

//We will assume that the signature is always at the end of the binary file
if (pID->VirtualAddress + pID->Size != szcbMem)
{
	//Signature is not at the end of file
	nOSErr = 1466;
	return XC_BadSignature;
}

Again, I had to use WinAPI Search to find the appropriate OS error code for this situation. I chose 1466, which is "An error was encountered while processing an XML digital signature".

I'll ignore the "XML" reference in our chosen OS error code message, as it is otherwise quite a good match for our error. Note that this message doesn't have to match exactly, as we are somewhat limited by the available system error codes. It just has to be close enough to clue in the user to what happened in that part of code.

Finally, when we are sure that we have a valid PE file, and that it has a signature, we can remove it. After that we will also need to adjust the checksum on the PE file itself. It is all done at the end of the process_PE_File function:

The checksum in the PE header is not widely used these days, but it is still a good practice to adjust it in case some legacy software relies on it.

//Set new file size
uicbNewFileSz = pID->VirtualAddress;

//Remove digital signature from the PE header directory
pID->Size = 0;
pID->VirtualAddress = 0;

//Update file checksum
DWORD dwOrigCheckSum = 0, dwNewCheckSum = 0;
PIMAGE_NT_HEADERS pIH = CheckSumMappedFile(pBaseAddr, uicbNewFileSz, &dwOrigCheckSum, &dwNewCheckSum);
if (!pIH)
{
	//Failed to compute new checksum
	nOSErr = ::GetLastError();
	return XC_FailedChecksum;
}

//Set new checksum in memory
assert(pdwChecksum);
*pdwChecksum = dwNewCheckSum;

return XC_Success;

We remove the digital signature by zeroing out the IMAGE_DIRECTORY_ENTRY_SECURITY and by cutting the actual signature data from the end of the binary file. This is done by setting uicbNewFileSz to the offset of the beginning of the signature section. That variable is returned from our function, and will be later used for saving an adjusted PE file.

Finally, we calculate and set the checksum after our modifications by calling the CheckSumMappedFile function that does all the work for us. All we have to do is just to store the resulting checksum in the appropriate location in the PE header (which we calculate earlier, and store in the pdwChecksum pointer, depending on the bitness of our PE binary.)

Video Overview - Part 2

Lastly, here's a full recording of what I described above. Make sure to play it full screen:

Finishing Touches, Handling Command Line

Next part centers around menial but necessary tasks in any production app. In our case I needed to code the basic about-page for the app that usually contains information about the author, copyright message and such.

I also needed to provide a basic help output and finally, make a command line parser so that users could instruct the app what to do.

Listing the code for these tasks would not be very interesting, so watch the video if you want to see the details.

One thing to note here is how I coded my command line parsing helper function. To account for the situation when user can specify command line parameters using a dash, or two types of slashes, as in -f, \f or /f; as well as by ignoring the case of command line parameters, I made the following function to check for all that in one place:

BOOL CSigRem::IsCmdLineParam(LPCTSTR pCmd, LPCTSTR pToCheck)
{
	//RETURN:
	//		= TRUE if 'pToCheck' is 'pCmd' command line parameter (case insensitive)

	if (pCmd &&
		pCmd[0] &&
		pToCheck &&
		pToCheck[0])
	{
		//Command must start with a letter only
		assert((pToCheck[0] >= 'a' && pToCheck[0] <= 'z') || (pToCheck[0] >= 'A' && pToCheck[0] <= 'Z') || pToCheck[0] == '?');

		WCHAR z = pCmd[0];
		if (z == '-' ||
			z == '/' ||
			z == '\\')
		{
			return ::CompareString(LOCALE_USER_DEFAULT, NORM_IGNORECASE, pCmd + 1, -1, pToCheck, -1) == CSTR_EQUAL;
		}
	}

	return FALSE;
}

Note that I'm using an assert check again. In this case this is needed to ensure that the caller of the IsCmdLineParam function doesn't use dashes or slashes in the pToCheck input parameter. And since that parameter will be hardcoded in our source code, this is just a debugging stage check.

Video Overview - Part 3

Lastly, here's a full recording of what I described above. Make sure to play it full screen:

Bug Fixes, Refactoring & Improvements

In part four of my coding session I will be addressing some bugs that I've discovered in the previous versions of the app, and will also introduce some new improvements to the app logic and will add some debugging checks.

Note that bugs are inherent for any software larger than the simple "Hello world" app. So it is a normal development process to find and eliminate bugs. Don't be alarmed by it!

What is important though is how a company addresses and fixes those bugs. The worst case scenario is when a company refuses to fix bugs, or even to acknowledge a bug's existence. They may also try to blame users for not "knowing how to use their software" instead of calling it a bug. As a user, an even as a developer, I would stay away from such a company.

I will also demonstrate why it is important to increment every production version of your app's build.

Incrementing a build version number of the app is primarily needed for identification of previous versions of the executable(s) for debugging purposes. So make sure to keep copies of not only the binaries and .pdb symbol files for each version of the app, but also a full copy of the source code files itself. Luckily, this is quite easy to do these days with any of the publicly available Git repositories, like Github and others.

I will also demonstrate how to refactor your existing code, in case you find a better suiting logic for the app's functionality. This part is very error prone, so please be careful when you do it. I will try to walk you through it in the video. I personally prefer to refactor my code manually instead of using any automated tools. This way I can control and proof-read the logic in the app as I go. But such approach makes refactoring much slower.

Lastly, I will add a special debugging check for the app. In it I will add a chunk of code that will be compiled only in the debugging configuration build. This code will check if our resulting PE file has a valid checksum. It will be present inside the RemoveDigitalSignature function:

Note that the original code in my video demonstration was slightly different. I adjusted it after I recorded it.

#ifdef _DEBUG
if (nResult == XC_Success)
{
	//Check that checksum was calculated correctly
	DWORD dwCheckSum1, dwCheckSum2;
	DWORD dwResChecksum = MapFileAndCheckSum(pStrOutputFile, &dwCheckSum1, &dwCheckSum2);
	assert(dwResChecksum == CHECKSUM_SUCCESS);
	assert(dwCheckSum1 == dwCheckSum2);
}
#endif

The important thing to note here is that I placed this entire section of my code within the _DEBUG preprocessor check. This means that it will be compiled and used only in the debugging configuration build, so don't place any production code there!

Video Overview - Part 4

Lastly, here's a full recording of what I described above. Make sure to play it full screen:

More Bug Fixes & Coding Test Fuzzer App

In the last part, I will explore the details of writing a separate test fuzzer app to subject my original SigRemover logic to some random input. The main reason for that is to test my PE file parser in the process_PE_File function for memory corruption bugs.

A fuzzer, is a small outside application that is designed to throw all kinds of random input into an app that is being tested. The purpose of such test is to see if the app crashes or creates some unexpected output. After that the fuzzer must have an option to record any such abnormalities for further debugging by the developers.

To write my testing app I chose a faster coding method using C# programming language, since our test fuzzer is not a production app and will technically stay for in-house use only.

The idea was to have the fuzzer enumerate through all files in the system drive and throw them as input into my SigRemover app logic. understandably, most of those files will not be valid PE files, and thus it would be a good way to test my PE parsing logic. It will also be a good way to test if the app parses valid PE files correctly as well.

Adjusting SigRemover Source Code For Fuzzing

But before I could do any fuzzing, I needed to adjust the source code in the SigRemover C++ app to account for it. I do so by defining a new preprocessor constant FUZZING_BUILD in the global Types.h file:

//#define FUZZING_BUILD			//Uncomment to generate a fuzzing build

As you can see, it will originally be commented out for regular builds of the app. And only when we need to run our app through the fuzzer, I will enable it by removing the comment in the global Types.h file before compiling the project.

Then the source code itself will be built depending on that constant.

First, we need to indicate it somewhere in the UI that this is a fuzzing build. This is important to ensure that it doesn't slip out into production! I did so by adding the following lines of code in the main() function:

int _tmain(int argc, WCHAR* argv[])
{
	//...

#ifdef FUZZING_BUILD
	wprintf(L"*** FUZZING BUILD ***\n");
#endif

As you can see, that will output a visible UI message for every fuzzing build of our app.

Then we need to use that preprocessor constant check to isolate and not compile any code of our application that deals with saving a new PE file. We won't need to do that since the code dealing with saving a PE file is not where we need to perform our tests. Plus, generating a new file on disk every time we run our test is not something that you would want to do. Such is done in the RemoveDigitalSignature function. (Check the full source code for that.)

Then, we also need to add a similar preprocessor constant check to the process_PE_File function where it checks for the presence of the IMAGE_NT_SIGNATURE magic sequence in the PE header, and returns an error if it doesn't find it:

//Define NT headers
IMAGE_NT_HEADERS* pNtHdr = (IMAGE_NT_HEADERS*)(pBaseAddr + (ULONG)pDosHdr->e_lfanew);
if (pNtHdr->Signature != IMAGE_NT_SIGNATURE)
{
#ifndef FUZZING_BUILD
	//Error
	nOSErr = ERROR_BAD_EXE_FORMAT;
	return XC_Not_PE_File;
#endif
}

It would be nice to skip this test when we run our fuzzer. This will provide us with a broader scope of possibilities for testing our random input.

Test Fuzzer App

Then, switching back to our C# .NET fuzzer app, we need to code a function to invoke our SigRemover C++ app with an arbitrary file. We can do it as such:

static void FuzzFile(string strFilePath, ref RemSigCountStats stats)
{
	try
	{
		Process proc = new Process();
		proc.StartInfo.FileName = @"..\..\..\..\Debug\SigRemover.exe";
		proc.StartInfo.Arguments = $"-i \"{ strFilePath }\"";
		proc.StartInfo.WindowStyle = ProcessWindowStyle.Hidden;

		proc.Start();

		proc.WaitForExit();

		EXIT_CODES exitCode = (EXIT_CODES)proc.ExitCode;

		//Check exit codes
		if(exitCode == EXIT_CODES.XC_BinaryHasNoSignature)
		{
			stats.nCount_NoSig++;
		}
		else if(exitCode == EXIT_CODES.XC_FailedToOpen)
		{
			stats.nCount_FailedToOpen++;
		}
		else if(exitCode == EXIT_CODES.XC_Not_PE_File)
		{
			stats.nCount_NotPE++;
		}
		else if(exitCode == EXIT_CODES.XC_Success)
		{
			stats.nCount_Success++;
		}
		else
		{
			//This needs our attention!
			Console.WriteLine($"Failed exit code: { exitCode } in file: { strFilePath}");	// *** Place breakpoint here ***
		}
	}
	catch(Exception ex)
	{
		Console.WriteLine($"Exception: { ex.ToString() }");		// *** Place breakpoint here ***
	}
}

Note that because I placed my SigRemFuzzer C# project in the same folder with the main SigRemover app, I can use a relative path to the SigRemover.exe executable from the C# app:

proc.StartInfo.FileName = @"..\..\..\..\Debug\SigRemover.exe";

This will avoid recoding the fuzzer, if we move the project to another location.

Note that because I kept all the exit codes from the SigRemover C++ app in an enum, I can easily copy-and-paste it into my C# fuzzer app, not worrying about missing any exit codes.

Then enumerating all files on a drive is quite easy in .NET:

static void EnumerateFolder(string strFldrPath, ref RemSigCountStats stats)
{
	if(!strFldrPath.EndsWith(@"\"))
		strFldrPath += @"\";

	string[] strFiles = null;
	try
	{
		strFiles = Directory.GetFiles(strFldrPath);
	}
	catch(UnauthorizedAccessException)
	{
		//Skip it
	}

	if (strFiles != null)
	{
		foreach (string strFile in strFiles)
		{
			Console.CursorTop = 0;
			Console.WriteLine(strFile + "\t\t\t\t\t\t\t\t\t");

			//Do the fuzzing of the file
			FuzzFile(strFile, ref stats);

			//Output stats
			Console.CursorTop = 10;
			Console.Write(
				$"Success:      {stats.nCount_Success}\n" +
				$"Not PE:       {stats.nCount_NotPE}\n" +
				$"Failed Open:  {stats.nCount_FailedToOpen}\n" +
				$"No signature: {stats.nCount_NoSig}\n"
				);
		}

		foreach (string strFldr in Directory.GetDirectories(strFldrPath))
		{
			EnumerateFolder(strFldr, ref stats);

		}
	}
}

The code above uses a simple counter class, that I defined as such:

class RemSigCountStats
{
	public int nCount_Success { get; set; }
	public int nCount_NotPE { get; set; }
	public int nCount_FailedToOpen { get; set; }
	public int nCount_NoSig { get; set; }
}

And then the sequence is initiated from the main() function to begin the fuzzing test:

static void Main(string[] args)
{
	RemSigCountStats stats = new RemSigCountStats();

	EnumerateFolder(@"C:\", ref stats);
}

Keep in mind that the fuzzing test may take many hours to complete (depending on the size of your system drive.)

Before running the test fuzzer though, I would suggest placing two breakpoints inside the FuzzFile function, where I indicated it with comments. Then run it with a .NET debugger and let the fuzzer run until the breakpoints hit. This will allow you to document the exit code from the SigRemover C++ app and the file that this situation happened with. Make sure to save it in a Notepad for the duration of your test. After that you can pass those files into the SigRemover C++ app with a C++ debugger attached to see where the issue happened.

That way (as I described above) I was able to find another bug, where I wasn't catching a new operator that was throwing an exception when I was expecting it to return a NULL instead. This prompted me to change a plain new operator into a non-throwing new (std::nothrow), as such:

BYTE* pFileMem = new (std::nothrow) BYTE[dwcbFileSz];
if(pFileMem)
{
	//Do work here ...

	//Free mem
	delete[] pFileMem;
	pFileMem = NULL;
}
else
	ReportOSError(ERROR_OUTOFMEMORY, L"Failed to reserve memory to read file: %s", pStrFilePath);

Note that I wouldn't have caught this bug had I not run my app through a test fuzzer.

Video Overview - Part 5

Lastly, here's a full recording of what I described above. Make sure to play it full screen:

Downloads

If you are interested in the SigRemover tool itself:

• You can download just its binary file here.
• Or, you can download its source code here as the Visual Studio 2019 solution.