How to Check Validity

How to Check Validity

When checking input for validity, you should follow this rule: look for valid data and reject everything else. The principle of failing securely, outlined in Chapter 3, Security Principles to Live By, means you should deny all access until you determine the request is valid. You should look for valid data and not look for invalid data for two reasons:

• There might be more than one valid way to represent the data.

• You might miss an invalid data pattern.

The first point is explained in more detail in Chapter 11, Canonical Representation Issues. It's common to escape characters in a way that is valid, but it's also possible to hide invalid data by escaping it. Because the data is escaped, your code might not detect that it's invalid.

The second point is very common indeed. Let me explain by way of a simple example. Imagine your code takes requests from users to upload files, and the request includes the filename. Your application will not allow a user to upload executable code because it could compromise the system. Therefore, you have code that looks a little like this:

bool IsBadExtension(char *szFilename) { bool fIsBad = false; if (szFilename) { size_t cFilename = strlen(szFilename); if (cFilename >= 3) { char *szBadExt[] = {".exe", ".com", ".bat", ".cmd"}; char *szLCase = _strlwr(_strdup(szFilename)); for (int i=0; i < sizeof(szBadExt) / sizeof(szBadExt[0]); i++) if (szLCase[cFilename-1] == szBadExt[i][3] && szLCase[cFilename-2] == szBadExt[i][2] && szLCase[cFilename-3] == szBadExt[i][1] && szLCase[cFilename-4] == szBadExt[i][0]) fIsBad = true; } } return fIsBad; } bool CheckFileExtension(char *szFilename) { if (!IsBadExtension(szFilename)) if (UploadUserFile(szFilename)) NotifyUserUploadOK(szFilename); }

What's wrong with the code? IsBadExtension performs a great deal of error checking, and it's reasonably efficient. The problem is the list of invalid file extensions. It's nowhere near complete; in fact, it's hopelessly lacking. A user could upload many more executable file types, such as Perl scripts (.pl) or perhaps Windows Scripting Host files (.wsh, .js and .vbs), so you decide to update the code to reflect the other file types. However, a week later you realize that Microsoft Office documents can contain macros (.doc, .xls, and so on), which technically makes them executable code. Yet again, you update the list of bad extensions, only to find that there are yet more executable file types. It's a never-ending battle. The only correct way to achieve the goal is to look for valid, safe extensions and to reject everything else. For example, in the Web file upload scenario, you might decide that users can upload only certain text document types and graphics, so the secure code looks like this:

bool IsOKExtension(char *szFilename) { bool fIsOK = false; if (szFilename) { size_t cFilename = strlen(szFilename); if (cFilename >= 3) { char *szOKExt[] = {".txt", ".rtf", ".gif", ".jpg", ".bmp"}; char *szLCase = _strlwr(_strdup(szFilename)); for (int i=0; i < sizeof(szOKExt) / sizeof(szOKExt[0]); i++) if (szLCase[cFilename-1] == szOKExt[i][3] && szLCase[cFilename-2] == szOKExt[i][2] && szLCase[cFilename-3] == szOKExt[i][1] && szLCase[cFilename-4] == szOKExt[i][0]) fIsOK = true; } } return fIsOK; }

As you can see, this code will not allow any code to upload unless it's a safe data type, and that includes text files (.txt), Rich Text Format files (.rtf), and some graphic formats. It's much better to do it this way. In the worst case, you have an annoyed user who thinks you should support another file format, which is better than having your servers compromised.