Security Errors in Code, Part 2

In Part 1I talked about the CWE and SANS 25 security coding errors. My contention was many of them are caused by lazy programming. In this part I'll be talking a bit more about lazy programming.

The best example of lazy programming I can think of is allocating memory without checking the result. Suppose you have a function that builds a message from a set of input strings, then displays the message. The function might look like this.

void DisplayMsg (char *domain, char *file) {
  char *b, *p = "Processing  from ";
  int d = strlen (domain);
  int f = strlen (file);
  int l = strlen (p);

  b = malloc (d + f + l + 1);
  memcpy (b, p, 11);
  memcpy (b + 11, file, f);
  memcpy (b + 11 + f, p + 11, 6);
  memcpy (b + 17 + f, domain, d + 1);

  DisplayString (b);
}

There are many examples of lazy code here, but let's start with the malloc. The question is, what if the malloc fails? Then b will be NULL and the first memcpy will crash the program. "But," declares the lazy programmer, "I've been programming for 8 years plus 4 years in college and I have never seen a malloc fail (except of course when there's a bug in the code somewhere). In the real world it's not going to fail, so why bother? Besides, what do I do if it does fail? There's nothing I can do."

The lazy programmer says, "There's nothing I can do." There are things you can do, it's just that the things to do require more work. So really what the lazy programmer is saying is, "There's nothing I can do that is quick and easy, I don't want to have to spend time writing code or expend energy using my brain to figure them out."

1. Have an "Out of Memory" message ready. If the malloc fails, display that.

  char *memoryMsg = "Out of Memory: Message Cannot be Displayed"

  if (b == NULL) {
    DisplayString (memoryMsg);
    return;
  }

2. Return an error. The subroutine DisplayMsg returns void, instead, have it return an error code.

  int DisplayMsg (char *domain, char *file) {
    ...
  if (b == NULL)
    return (M_ERROR_MEMORY);

The lazy programmer says, "But handling errors is hard. Now the calling routine has to deal with an error." Yes, it's hard. But it's better than dealing with bad code. Besides, that's what you're paid to do, write good code and solve the hard problems.

I think any programmer who writes code like the above is doing it that way because it takes effort to write good code. And now that we know this programmer does indeed prefer to take the easy route and hope nothing bad happens, we will be suspicious of all code they produce.

By the way, there are other examples of lazy programming in the example above. See what you find and in Part 3 I'll discuss what I think is lazy.