Many people tout certain APIs as dangerous. Although it is true that some function calls can have insecure ramifications if used incorrectly, we have learned that simply banning, outlawing, or discouraging the use of certain functions is helpful but not sufficient to produce more secure code. Rather, it creates a false sense of security. As in the off-by-one example in Chapter 5, even the safer functions can cause exploitable problems when used incorrectly. However, a number of software projects have obtained measurable gains in security by banning functions that are difficult to use safely.
Dave Cutler, Microsoft’s chief architect of Microsoft Windows NT, once told me there are no such things as dangerous functions, only dangerous developers. He is correct. That said, you should be aware of the side effects and nuances of certain functions, and this appendix outlines some of the more common ones. Let’s think about this for a moment: some developers are dangerous on most days and should probably be encouraged to take up a different line of work, perhaps program management! A precious few developers are dangerous one day out of 100. The rest of us will tend to do better using functions and classes that make it harder for us to make mistakes. In addition to using functions that lead to mistakes less often, a deep understanding of the functions you use will also reduce mistakes.
The most important thing to understand is that most security issues result from trusting input. It is imperative that you trace data as it comes into your code and question the implications of operations on that data. You can write secure code by using most so-called insecure functions, as long as the data is well-formed and trusted.
Important
Do not replace "insecure" functions with "secure" functions and expect to ship a secure product. You need to follow the data through your code and question the trustworthiness and correctness of that data as it is manipulated by the code.
Many functions exist in the C run time and within operating systems that when used incorrectly might lead to buffer overruns. The following sections describe some of our "top picks."
These functions do not check the size of the destination buffer and do not check for null or otherwise invalid pointers. If the source buffer is not null-terminated, results are indeterminate. Strongly consider using the "n" versions or strsafe instead.
Important
Simply using the "n" versions or strsafe does not make your code secure; you must still validate that the data is well-formed and trusted prior to copying it to another buffer.
These functions do not check the length of the destination buffer and do not check for null or otherwise invalid pointers. If the source buffer is not null-terminated, results are indeterminate. Strongly consider using the "n" versions or strsafe instead.
It’s not guaranteed that these functions will null-terminate the destination buffer, and they do not check for null or otherwise invalid pointers.
Check that the number of characters to be copied is the number of characters remaining in the buffer, not the size of the buffer. These functions depend on the source buffers and destination buffers being null-terminated.
The destination buffer must be large enough to hold the number of bytes specified in the length argument. Otherwise, you might get buffer overruns. Consider using _memccpy if you know that the code should copy only to a specified character.
These functions are not guaranteed to null-terminate the destination buffer. Unless field widths are strictly defined, these functions are very difficult to use safely. Consider using StringCchPrintf instead.
These functions might not null-terminate the destination buffer. Also they pose cross-platform compatibility issues because return behavior (and termination behavior) varies with the platform. Consider using StringCchPrintf instead.
This family includes printf, _sprintf, _snprintf, vprintf, vsprintf, and the wide character variants of these functions. Ensure that user-defined strings are not passed as the format string. Also, use of implicit wide character to single-byte conversion via the %s specifier might result in the resulting string having fewer characters than the input string. If you want to control this behavior, use the WideCharToMultiByte function.
Also, be wary of format strings that have a dangling %s—for example, sprintf(szTemp, "%d, %s", dwData, szString)—because the last argument is as bad as an unbounded strcpy. Use the _snprintf or StringCchPrintf functions instead.
None of these functions handles buffers that are not null-terminated properly. Calling them will not lead to exploitable buffer overruns, but they might lead to access violations if the function attempts to read into "no-man’s-land." Consider using exception handlers around such code if the data comes from an untrusted source. StringCchLength offers a safer mechanism.
The gets function is plain evil. You cannot write a secure application that uses this function because it does not check the size of the buffer being copied. Ban its use. Use fgets instead. Another approach is to use getc in a loop and check bounds.
Like gets, scanf, _tscanf, and wscanf when using %s are hard to get correct because %s is unbounded. You can certainly limit the size of the string by using constructs such as %32s; better to use fgets.
The C++ Standard Template Library (STL) stream operator (>>) copies data from an input source to a variable. If the input is untrusted, this could potentially lead to a buffer overrun. For example, the following code takes input from stdin (cin) and passes it to szTemp, but a buffer overrun occurs if the user enters more than 16 bytes:
#include "istream"
void main(void) {
char szTemp[16];
cin >> szTemp;
}It’s just as bad as gets. Use alternate functions or restrict the input data size by using cin.width.
The last argument to this function is the number of wide characters in the string, not the number of bytes. If you pass in the number of bytes, you are indicating that the buffer is actually twice as large. The following code is incorrect:
WCHAR wszName[NAME_LEN]; MultiByteToWideChar(…,…,…,…,sizeof(wszName));
The last argument should read sizeof(wszName)/sizeof(wszName[0]) or simply NAME_LEN, but don’t forget to accommodate for the trailing termination character if appropriate.
These functions manipulate multibyte—most commonly, double-byte—characters and can cause errors when dealing with malformed data, such as a lead byte followed by zero instead of a valid trail byte. You can determine leading-a-trailing-byte validity by using the isleadbyte, _ismbslead, and _ismbstrail functions. Also, _mbbtype is a useful function.
Any API call that can create something with a name is prone to name-squatting issues. There are two parts to the problem. The first is that the attacker would like to guess what file or other object is being created and then create it before you get there. For example, if when I edit a file the editor creates a file with a predictable name in c: emp, an attacker could precreate the file with permissions that allow her to read it and then manipulate my file. Another attack is to link to a file I don’t have write access to and then get an administrator to delete it for me or, worse yet, to change the permissions. The solution to most of these types of attacks is to use per-user temporary space, located in the user’s Documents And Settings folder, assuming that we’re dealing with Microsoft Windows 2000 or later. If you have to create temporary files or directories in a public area, the best approach is to generate a truly random name for your object. The second part of the solution, when creating files, is to use the CREATE_NEW flag, which will cause the function to fail if the file already exists.
Never assume that a file or other object does not exist if you checked to determine whether it was present. There is a window of opportunity between checking for existence and creation in which an attacker can exploit you. The timing might be tight and you might think that the chances of pulling off such an attack are miniscule, but think again! Numerous potent attacks have been made against UNIX systems that were vulnerable to race conditions, and a few we’ve seen have affected Windows as well. Windows isn’t any less vulnerable—it just typically doesn’t have multiple local users at once, unless you’re running Terminal Services.
Named pipes have another set of issues, which is that the owner of a named pipe can often impersonate the client, depending on how the client opened the pipe. If the client is a high-level process, this can lead to escalation of privilege. One way to defend yourself against this attack is to open your pipe with the FILE_FLAG_FIRST_PIPE_INSTANCE flag set. Note that this works only on Windows 2000 SP1 and later. This is covered in detail in Chapter 23.
Here’s one approach that overcomes the escalation of privilege attack: when your server starts, it generates a random name, creates the pipe with that name, and stores the name in a registry key that is writable only by administrators. Clients then check the registry key to determine the pipe to open. If the server exits, it clears the value in the registry. Although this is a good method, attacks still exist if your server can be caused to exit without clearing the stored pipe name.
If your server exposes either RPC interfaces or named pipes across the network, the clients are going to depend on a particular interface ID or pipe name existing on the server. The best bet in these instances is to ensure that your service starts as early as possible in the boot order.
Some functions, when used incorrectly, could lead to an application loading unintended code. Admittedly, this does mean the attacker has loaded malicious data on the computer being attacked, so you should consider this section as "good hygiene" and one concerned with defense in depth.
The first argument is the application path; the second is the command line. If the first argument is null and the second argument has white space in the application path, unintended applications could be executed. For example, if the argument is c:Program FilesMyAppMyApp.exe, c:Program.exe could be executed. A workaround is to specify the application path in the first argument or to double-quote the application path in the second argument.
These functions behave like CreateProcess(NULL,…) and should be used with extreme caution.
On many versions of the Windows operating system, the current directory is searched first when loading files. If you attempt to load a DLL by using a non-fully-qualified path (for example, file.dll rather than c:dirdirfile.dll), the code will look in the current directory first for the code, and if there’s a malicious file in the "." directory it is loaded first. It’s recommended that you always use a full path when using these functions.
Suggestions: If your DLLs are installed with the rest of your application, store your installation directory in the registry and use this to specify a full path to the DLL. If the DLL is stored in a directory owned by the operating system, use GetWindowsDirectory to find the correct DLL. Note issues with systems running Terminal Services.
These are a nonissue in Windows XP SP1 and later and Microsoft Windows .NET Server 2003 because the path is searched differently. The system directories are searched first, followed by the current directory.
Just about everything on the Windows desktop is a window, right down to the scroll bar. Because windows can have different styles and types, some of these messages have potential security ramifications. Sending messages requires that the developer (or attacker) knows the window handle (hWnd) and sends the message by using SendMessage. The following sections describe the most dangerous Windows styles and control types.
These messages copy data from a control into a buffer; make sure lParam is set to NULL first to acquire the source buffer size first.
There is no way to limit the size of the buffer; it assumes the source is no more than 80 characters long. Be careful when using this message.
In general, you should always use the GETTEXTLENGTH message first to determine the size of the source string. However, if the size of the data changes between determining the length and you copying the data by using the appropriate get text message, you might still have a buffer overrun. Be very conservative when calling these.
There is presently no way to query the text length of a ToolTip text from a status bar with SB_GETTIPTEXT.
This edit control window style displays all characters as an asterisk (*) as they are typed. Remember to erase the buffer you passed to GetWindowText or SetWindowText so that the password doesn’t reside in cleartext in memory. Refer to Chapter 9, for more information.
If a call to an impersonation function fails 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 SYSTEM, or as a member of an administrative group, the user might be able to perform actions he would otherwise be disallowed. Therefore, it’s important that you always check the return value of the call. If it fails to raise an error, do not continue execution of the client request. Impersonation functions include RpcImpersonateClient, ImpersonateLoggedOnUser, CoImpersonateClient, ImpersonateNamedPipeClient, ImpersonateDdeClientWindow, ImpersonateSecurityContext, ImpersonateAnonymousToken, ImpersonateSelf, and SetThreadToken.
Also, in Microsoft Windows .NET Server 2003, impersonation is a privilege and is not granted to everyone. This increases the chance your code may not successfully impersonate an account. Impersonation works in Windows .NET Server 2003 if one or more of the following conditions are true:
The requested impersonation level is less than impersonate (that is, anonymous or identify level, which should always succeed).
The process token has SeImpersonatePrivilege.
This process (or another process in this logon session) created the token via LogonUser with explicit credentials.
This token is for the current application user.
The application is a COM or COM+ server started via COM activation services, because the Service SID is added to the application’s primary token by COM. This does not include COM applications started as Activate as Activator.
Creating security descriptors that have a NULL DACL—that is, pDacl, the third argument, 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, thereby denying everyone, including administrators, access to the object. A NULL DACL offers absolutely no protection from attack.
The APIs in the following sections can lead to a denial of service condition, particularly under low memory conditions.
These functions can throw exceptions in low-memory situations and if the exception is not caught, the application will halt. Consider using InitializeCriticalSectionAndSpinCount instead. Note that EnterCriticalSection will not throw exceptions under Windows XP, Windows .NET Server, and later. Also, be careful not to make blocking networking calls from within a critical section or while holding any other type of lock. Finally, any code within a critical section should be examined carefully. Any exceptions thrown should be caught within the critical section, or you’ll end up in an exception handler without calling LeaveCriticalSection. Do the absolute minimum required within a critical section. One way around this when dealing with C++ code is to create a lock object that calls LeaveCriticalSection when the stack unwinds.
_alloca allocates memory on the stack and is freed when the function exits, assuming there is enough memory! In many instances, this function will throw an exception, which if unhandled will halt the process. Be careful of macros that wrap _alloca, such as the ATL character-mapping macros, including A2OLE, T2W, W2T, T2COLE, A2W, W2BSTR, and A2BSTR.
The most generic observation with _alloca is that you should wrap the call in an exception handler and you should not allocate memory based on a size determined by the user.
Finally, you should call _resetstkoflw in the exception handler; this function recovers from a stack overflow condition, enabling a program to continue instead of failing with a fatal exception error. The following sample shows the process:
#include "malloc.h"
#include "windows.h"
...
void main(int argc, char **argv) {
try {
char *p = (char*)_alloca(0xfffff);
} __except(GetExceptionCode() == STATUS_STACK_OVERFLOW) {
int result = _resetstkoflw();
}
}Both of these functions should be called only in an emergency situation. This is especially true with TerminateThread. Any memory, handles, and system resources owned by the thread in question will not get cleaned up. To quote from the Platform SDK:
"TerminateThread is a dangerous function that should only be used in the most extreme cases. You should call TerminateThread only if you know exactly what the target thread is doing, and you control all of the code that the target thread could possibly be running at the time of the termination."
The only time it is appropriate to call TerminateThread is if the application is shutting down and one or more threads are not responding. TerminateProcess does not clean up global data owned by DLLs, and most applications should call ExitProcess, unless the process being terminated is external. For those of you used to UNIX systems, TerminateProcess does not clean up resources used by child processes of the parent process. The whole notion of parent and child processes is not fully implemented on Win32 systems.
A network is a very hostile place, and making assumptions about whether a connection is still valid can get you into trouble. Never make networking calls from inside a critical section if you can possibly avoid it. All sorts of things can go wrong, ranging from a connection being dropped before you have a chance to send, to a malicious client setting a miniscule TCP window size.
Be careful when binding to INADDR_ANY (all interfaces)—you might be at risk of socket hijacking. See Chapter 15, for details.
This function has a trinary return, and all three possibilities aren’t always trapped. An error is -1, a graceful disconnect (or end of buffer) returns 0, and a positive number indicates success. In general, it’s a bad idea to call recv using a blocking socket. Under certain error conditions, a blocking recv can hang a thread indefinitely. For high performance, use WSAEventSelect. It may not be portable, but the performance gains are worth it.
This function sends data to a connected socket. Do not assume that all the data was successfully transmitted if send succeeded. Connections sometimes drop between the call to connect and the send. Additionally, if someone is maliciously setting your TCP window size to a very small value, the only way you’ll notice it will be if the send call starts to time out. If you have the socket set to blocking or don’t check the return from this function, you’ve just opened yourself up to a denial of service condition.
These calls are tremendously useful and return all sorts of information about Windows systems. Examples include NetUserGetInfo, NetShareEnum, etc. Unfortunately, they are all blocking calls. If you need these calls, plan on working around the fact that they will block, usually for 45 seconds, sometimes longer. A second caveat is that if you end up dealing with non-Microsoft SMB (Server Message Block) implementations, you could get unusual behaviors. For example, a Microsoft system might always give you a valid pointer if it succeeds, but a non-Microsoft system might give you a NULL pointer. Just as a server should never assume a benign client, a client application should never assume a well-behaved server.
This section is a catchall for APIs that cannot be pigeonholed into another category.
The main reason for not using the IsBadXXXPtr functions is they encourage developers to be sloppy and use unchecked pointers. These functions are a legacy from 16-bit Windows, and their use is discouraged in new code. In most cases, it’s sufficient to check for a NULL pointer. For other scenarios, you should wrap the pointer code in a structured exception handler (SEH). Be aware that this is still a dangerous proposition if the exception handler is corrupted because of a buffer overrun while copying untrusted data. Do not catch all exceptions in your exception handler; only handle the exceptions you know about, such as STATUS_ACCESS_VIOLATION.
Of course, if you catch an exception in your code, you have a bug that needs fixing!
These functions do not guarantee that the memory pointed to is valid or safe to use. Consider calling IsBadWritePtr on a stack-based buffer. The function will indicate it is safe to use the memory, but we all know it probably is not. Because of the multitasking nature of Windows, nothing is preventing another thread from changing the memory protection between your code testing the page and the application using the page.
Important
You should never manipulate a pointer not under the direct control of your application.
Finally IsBadWritePtr is not thread-safe!