The Sin Explained

There are six variants of this sin:

• Yielding too much information

• Ignoring errors

• Misinterpreting errors

• Using useless error values

• Handling the wrong exceptions

• Handling all exceptions

Lets look at each in detail.

Yielding Too Much Information

We talk about this issue in numerous places in the book, most notably in Sin 13. Its a very common issue: an error occurs and, in the interest of usability, you tell the user exactly what failed, why, and, often, how to fix the issue. The problem is you just told the bad guy a bunch of really juicy information, toodata he can use to compromise the system.

Ignoring Errors

Error return values are there for a very good reason: to indicate a failure condition so your code can react accordingly . Admittedly, some errors are not serious errors; they are informational and often optional. For example, the return value of printf is very rarely checked; if the value is positive, then the return indicates the number of characters printed. If its 1, then an error occurred. Frankly, for most code, its not a big issue.

For some code, the return value really does matter. For example, Windows includes many impersonation functions, such as ImpersonateSelf(), ImpersonateLogonUser(), and SetThreadToken(). If these fail for any reason, then the impersonation failed and the thread still has the identity associated with the process.

Then theres file I/O. If you call a function like fopen(), and it fails (access denied or no file), and you dont handle the error, subsequent calls to fwrite() or fread() fail too. And if you read some data and dereference the data, the application will probably crash.

In languages with exception handling, exceptions are the primary way to pass errors. Languages like Java try to force the programmer to deal with errors by checking to ensure they catch exceptions at compile time (or, at least, delegate responsibility for catching the exception to the caller). There are some exceptions, however, that can be thrown from so many parts of the program that Java doesnt require they be caught, particularly the NullPointerException. This is a pretty unfortunate issue, since the exception getting thrown is usually indicative of a logic error; meaning that, if the exception does get thrown, it is really difficult to recover properly, even if you are catching it.

Even for the errors Java does force the programmer to catch, the language doesnt force them to be handled in a reasonable manner. A common technique for circumventing the compiler is to abort the program without trying to recover, which is still a denial of service problem. Even worse , but sadly much more common, is to add an empty exception handler, thus propagating the error. More on this later.

Misinterpreting Errors

Some functions, such as the sockets recv() function, are just weird. recv() can return three values. Upon successful completion, recv() returns the length of the message in bytes. If no messages are available to be received and the peer has performed an orderly shutdown, recv() returns 0. Otherwise, - 1 is returned and errno is set to indicate the error.

Using Useless Error Values

Some of the C standard run-time functions are simply dangerousfor example, strncpy , which returns no error value, just a pointer to the destination buffer, regardless of the state of the copy. If the call leads to a buffer overrun , the return value points to the start of the overflowed buffer! If you ever needed more ammunition against using these dreaded C run-time functions, this is it!

Handling the Wrong Exceptions

In languages that handle exceptions, you should be careful to handle the correct exceptions. For example, at the time of writing, the C++ standard dictates that:

The allocation function can indicate failure by throwing a bad_alloc exception. In this case, no initialization is done.

The problem is, in some cases, this is not always the case. For example, in the Microsoft Foundation Classes, a failed new operator can throw a CMemoryException, and in many modern C++ compilers (Microsoft Visual C++ and gcc, for example), you can use std::nothrow to prevent the new operator from raising an exception. So if your code is written to handle, say a FooException, but the code in the try/catch only throws a BarException, then your code will simply die when it throws a BarException, because there is nothing there to catch it. Of course, you could catch all exceptions, but thats simply bad too, and thats the next topic.

Handling All Exceptions

What were about to explain in this sectionhandling all exceptionsis 180 different from the title of the Sin, Failing to Handle Errors, but it is very much related . Handling all exceptions is just as bad as failing to handle errors. In this case, your code gobbles up errors it doesnt know about, or cannot handle, or worse simply masks errors in the code, and thats the problem. If you hide errors by simply pretending the exception never happened , you may have latent bugs you dont know of that eventually surface when some code dies in weird and wonderful ways that are often impossible to debug.

Sinful C/C++

In the code sample that follows , the developer is checking the return from a function that yields a completely useless error valuethe return from strncpy is a pointer to the start of the destination buffer. Its of zero use.

 char dest[19]; char *p = strncpy(dest, szSomeLongDataFromAHax0r,19); if (p) {  // everything worked fine, party on dest or p } 

The variable p points to the start of dest, regardless of the outcome of strncpy, which, by the way, will not terminate the string if the source data is equal to, or longer than, dest. Looking at this code, it looks like the developer doesnt understand the return value from strncpy; theyre expecting a NULL on error. Oops!

The following example is common also. Sure, the code checks for the return value from a function, but only in an assert, which goes away once you no longer use the debug option. Also there is no validity checking for the incoming function arguments, but thats another issue altogether.

 DWORD OpenFileContents(char *szFilename) {  assert(szFilename != NULL);  assert(strlen(szFilename) > 3);  FILE *f = fopen(szFilename,"r");  assert(f);  // Do work on the file  return 1; } 

Sinful C/C++ on Windows

As we mentioned earlier, Windows includes impersonation functions that may fail. In fact, since the release of Windows Server 2003 in 2003, a new privilege was added to the OS to make impersonation a privilege granted only to specific accounts, such as service accounts (local system, local service, and network service) and administrators. That simply means your code could fail when calling an impersonation function, as shown:

 ImpersonateNamedPipeClient(hPipe); DeleteFile(szFileName); RevertToSelf(); 

The problem here is if the process is running as Local System, and the user calling this code is simply a low-privileged user, the call to DeleteFile may fail because the user does not have access to the file, which is what you would probably expect. However, if the impersonation function fails, the thread is still executing in the context of the process, Local System, which probably can delete the file! Oh no, a low-privileged user just deleted the file.

The next example is an example of handling all exceptions. Windows includes structured exception handling (SEH) for all programming languages:

 char *ReallySafeStrCopy(char *dst, const char *src) {  __try {  return strcpy(dst,src);  } __except(EXCEPTION_EXECUTE_HANDLER) {      // mask the error  }  return dst; } 

If strcpy fails because src is larger than dst, or src is NULL, you have no idea what state the application is in. Is dst valid? And depending on where dst resides in memory, what state is the heap or stack? You have no clueand yet the application will probably keep running for a few hours until it explodes. Because the failure happens so much later than the incident that caused the error, this situation is impossible to debug. Dont do this.

Sinful C++

In the code that follows, the new operator will not throw an exception, because your code tells the compiler to not throw! If the new fails and you come to use p, the app simply dies.

 try {  struct BigThing { double _d[16999];};  BigThing *p = new (std::nothrow) BigThing[14999];  // Use p } catch(std::bad_alloc& err) {  // handle error } 

In this example, the code is expecting to catch a std::bad_alloc exception, but its using the Microsoft Foundation Classes, which throws a CMemoryException.

 try {  CString str = new CString(szSomeReallyLongString);  // use str } catch(std::bad_alloc& err) {  // handle error } 

Sinful C#, VB.NET, and Java

The pseudo-code example that follows shows how not to catch exceptions. The code is catching every conceivable exception, and like the Windows SEH example, could be masking errors.

 try {  // (1) Load an XML file from disc  // (2) Use some data in the XML to get a URI  // (3) Open the client certificate store to get a   // client X.509 certificate and private key  // (4) Make an authenticated request to the server described in (2)   // using the cert/key from (3) } catch (Exception e) {  // Handle any possible error  // Including all the ones I know nothing about } 

All the functionally in the preceding code includes a dizzying array of possible exceptions. For .NET code, this includes the following: SecurityException, XmlException, IOException, ArgumentException, ObjectDisposedException, NotSupportedException, FileNotFoundException, and SocketException. Does your code really know how to handle all these exceptions correctly?

Dont get me wrong. It may be perfectly valid to catch all exceptions, but please double-check that it really is the right thing to do.

Related Sins

This sin stands alone. There really are no related sins; however, the first variant of this sin is discussed in more detail in Sin 13.