A Great Step Forward in Application Security: Documenting Security Implications of C Runtime and Windows APIs

  • Article

 

Michael Howard
Security Program Manager
Secure Windows Initiative Team
Windows XP Team
Microsoft Corporation

April 2001

Summary: This article discusses common mistakes made using function calls with C and C++ and their security ramifications and outlines how certain functions should be used. This is an ongoing effort and over the next few months we'll be augmenting numerous APIs with security information. (6 printed pages)

Introduction

While performing code reviews of C and C++ code to look for security vulnerabilities, I have come across common mistakes in the way certain function calls are used. Even though a function call may not be security-related, it may have subtle security ramifications if incorrectly used.

This article will discuss these mistakes and their security ramifications and will outline how certain functions should be used.

We have started a process of discussing security remarks in some APIs documented in MSDN and Platform SDK (this is an ongoing exercise) and in the first round of discussions we have outlined the top function calls that have led to security vulnerabilities in both Microsoft and non-Microsoft products.

To begin with, the following functions have additional security remarks:

CopyMemory CreateProcess, CreateProcessAsUser, CreateProcessWithLogonW SetSecurityDescriptorDacl Impersonation Functions memcpy sprintf, swprintf strcat, wcscat, _mbscat strcpy, wcscpy, _mbscpy strncat, wcsncat, _mbsncat WinExec Conclusion

CopyMemory

Security Remarks

The first argument, Destination, must be large enough to hold count bytes of Source combined ** otherwise a buffer overrun may occur. This may lead to a denial of service attack against the application if an access violation occurs, or in the worst case, allow an attacker to inject executable code into your process. This is especially true if Destination is a stack-based buffer. Be aware that the last argument, Length, is the number of bytes to copy into Destination, not the size of the Destination.

The following code example shows a safe way to use CopyMemory():

void test(char *pbData, unsigned int cbData) {
    char buf[BUFFER_SIZE];
    CopyMemory(buf, pbData, min(cbData,BUFFER_SIZE));
}

CreateProcess, CreateProcessAsUser, CreateProcessWithLogonW

Security Remarks

The first parameter, lpApplicationName, can be NULL, in which case the executable name must be the first white space-delimited string in lpCommandLine. If the executable or path name has a space in it however, there is a risk that a malicious executable could be run if the spaces are not properly handled. The following example is dangerous since the process will attempt to run "Program.exe," if it exists, instead of "foo.exe."

CreateProcess(NULL, "C:\Program Files\foo", ...)

If a malicious user were to create a Trojan program called "Program.exe" on a system, any program that incorrectly calls CreateProcess using the Program Files directory will now launch the Trojan instead of the intended application.

Consider not passing NULL for lpApplicationName to avoid the function having to parse and determine the executable pathname from its runtime parameters. Otherwise, use quotations around the executable path in lpCommandLine, if lpApplicationName is NULL, as shown in the example below.

CreateProcess(NULL, "\"C:\Program Files\foo.exe\" -L -S", ...)

SetSecurityDescriptorDacl

Security Remarks

Creating security descriptors that have a NULL DACL (for example, pDacl is NULL) is highly discouraged. Such a DACL offers no security for the object. Indeed, an attacker can set an Everyone (Deny All Access) ACE on the object denying everyone, including administrators, access to the object. A NULL DACL offers absolutely no protection from attack.

Impersonation Functions

Security Remarks

If the calls to an impersonation function fail for any reason, the client is not impersonated and the client request is made in the security context of the process from which the call was made. If the process is running as a highly privileged account, such as LocalSystem, or as a member of an administrative group, the user may be able to perform actions they would otherwise be disallowed. Therefore it is important that you always check the return value of the call, and if it fails to raise an error, do not continue execution of the client request. Examples include:

RpcImpersonateClient
ImpersonateNamedPipeClient
SetThreatToken
ImpersonateSelf
CoImpersonateClient
ImpersonateDdeClientWindow
ImpersonateSecurityContext
ImpersonateLoggedOnUser

memcpy

Security Remarks

The first argument, dest, must be large enough to hold count bytes of src combined, ** otherwise a buffer overrun may occur. This may lead to a denial of service attack against the application if an access violation occurs, or in the worst case, allow an attacker to inject executable code into your process. This is especially true if dest is a stack-based buffer. Be aware that the last argument, count, is the number of bytes to copy into dest, not the size of the dest.

The following code example, shows a safe way to use memcpy():

void test(char *pbData, unsigned int cbData) {
    char buf[BUFFER_SIZE];
    memcpy(buf, pbData, min(cbData,BUFFER_SIZE));
}

sprintf, swprintf

Security Remarks

The first argument, buffer, must be large enough to hold the formatted version of format and the trailing NULL ('\0') character ** otherwise a buffer overrun may occur. This may lead to a denial of service attack against the application if an access violation occurs, or in the worst case, allow an attacker to inject executable code into your process. This is especially true if buffer is a stack-based buffer.

Also be aware of the dangers of a user or application providing format as a variable. The following example is dangerous because the attacker may set szTemplate to "%90s%10s" which will create a 100-byte string:

void test(char *szTemplate,char *szData1, char *szData2) {
    char buf[BUFFER_SIZE];
    sprintf(buf,szTemplate,szData1,szData2);
}

Consider using _snprintf or _snwprintf instead.

strcat, wcscat, _mbscat

Security Remarks

The first argument, strDestination, must be large enough to hold the current strDestination and strSource combined ** and a closing '\0', otherwise a buffer overrun may occur. This may lead to a denial of service attack against the application if an access violation occurs, or in the worst case, allow an attacker to inject executable code into your process. This is especially true if strDestination is a stack-based buffer. Consider using strncat, wcsncat, or _mbsncat.

strcpy, wcscpy, _mbscpy

Security Remarks

The first argument, strDestination, must be large enough to hold strSource and the closing '\0', otherwise a buffer overrun may occur. This may lead to a denial of service attack against the application if an access violation occurs, or in the worst case, allow an attacker to inject executable code into your process. This is especially true if strDestination is a stack-based buffer. Consider using strncpy, wcsncpy, or _mbsncpy.

strncat, wcsncat, _mbsncat

Security Remarks

The first argument, strDestination, must be large enough to hold the current strDestination and strSource combined ** and a closing NULL ('\0'), otherwise a buffer overrun may occur. This may lead to a denial of service attack against the application if an access violation occurs, or in the worst case, allow an attacker to inject executable code into your process. This is especially true if strDestination is a stack-based buffer. Be aware that the last argument, count, is the number of bytes to copy into strDestination, not the size of the strDestination.

Note also that strncat only adds a trailing NULL if there is room left in the buffer, strDestination.

The following code example, shows a safe way to use strncat:

void test(char *szWords1, char *szWords2) {
     char buf[BUFFER_SIZE];
      
     strncpy(buf,szWords1,sizeof buf - 1);
     buf[BUFFER_SIZE - 1] = '\0';                
     unsigned int cRemaining = (sizeof buf - strlen(buf)) - 1;
     strncat(buf,szWords2,cRemaining);
}

WinExec

Security Remarks

The executable name is treated as the first white space-delimited string in lpCmdLine. If the executable or path name has a space in it however, there is a risk that a malicious executable could be run if the spaces are not properly handled. The following example is dangerous since the process will attempt to run "Program.exe," if it exists, instead of "foo.exe."

WinExec("C:\Program Files\foo", ...)

If a malicious user were to create a Trojan program called "Program.exe" on a system, any program that incorrectly calls WinExec using the Program Files directory will now launch the Trojan instead of the intended application.

From a security perspective, it is highly recommended you use CreateProcess rather than WinExec. However, if you must use WinExec for legacy reasons, make sure the application name is bounded by quotation marks as shown in the example below:

WinExec("\"C:\Program Files\foo.exe\" -L -S", ...)

Conclusion

Over time, we will add to this list of functions as we learn more about how some of them can be used incorrectly. If you have any comments on these function calls, please feel free to e-mail me at mikehow@microsoft.com.