, Matasano 2008-04-18
This is all my fault.
Many moons ago, I wrote a blog post chiding developers for checking the return value of malloc, the C function that allocates chunks of memory for programs to work with. When malloc fails, it returns NULL. According to Hoyle, youre meant to check for that value, because malloc can fail at absolutely any time (you are not the only program claiming memory).
I stand by that argument, and by most of the wording of that blog post. Now about that word most.
Dave LeBlanc and I go back, though he may not remember that. Last bubble, we were dev leads on competing products. Weve taken different career paths, and, long story short, hes technically now more of an authority on secure coding than I am. And Im telling you this because LeBlancs response to my last post is - faithful paraphrase - are you high?
LeBlanc thinks youd have to be not check malloc returns, because:
not checking will inevitably crash the program, and crashes are bad,
not checking leads to the bug class Dowd found, and
not checking leads to the bug class Dowd found.
LeBlanc is right. I am wrong. Not checking is bad. Let me make a very slight semantic adjustment, so that I might be inassailably correct (again).
Here are three (extremely contrived) code examples. The first, lets call, unchecked. It simply doesnt check the return of malloc.
#define hostile /**/
void *
_setup(unsigned hostile slot, unsigned hostile id) {
u_int32_t *slots = malloc(SLOTS_SIZE);
slots[slot] = id; // XXX write32 corruption
return slots;
}
The second, well call caller-checks. As you can see, it does.
#define hostile /**/
void *
_setup(unsigned hostile slot, unsigned hostile id) {
u_int32_t *slots = NULL;
if((slots = malloc(SLOTS_SIZE)))
slots[slot] = id;
return slots;
}
Now the third, which looks suspiciously like the first, well callee-checks.
#define hostile /**/
void *
_setup(unsigned hostile slot, unsigned hostile id) {
u_int32_t *slots = malloc(SLOTS_SIZE)))
// NOT REACHED
slots[slot] = id;
return slots;
}
Whats the difference between the first and the third? In the third, if malloc fails, it does not return NULL. It instead hands the program off to a recovery regime, which, by default, safely and immediately ends the program.
Whats the difference between caller-checks and callee-checks?
First, callee-checks is safer. You cant screw it up. The worst you can do is write a program that will abruptly terminate. This is far better than the current worst-case scenario, in which manifestly common programmer errors allow Mark Dowd to upload malicious code into your program.
Second, callee-checks is cleaner. In the caller-checks case, not only does setup need to check, but so does setups caller, and its callers caller, and its callers callers caller, all the way down to the place where your program inevitably gives up and terminates the program.
But, Thomas, you say, not all programs do give up and abort. Some have policies for handling out-of-memory conditions. And so they do. And in most cases, those policies are global, and can simply be substituted for the default behavior of exiting the program.
But I will grant you that in many cases, you have a genuinely useful recovery regime that is specific to one code-path - say, an arena-style allocation regime for a particular user request - and no global policy will help. So, I submit to you a fourth option, which I will not name, and which looks suspiciously like example (2):
#define hostile /**/
void *
_setup(unsigned hostile slot, unsigned hostile id) {
u_int32_t *slots = NULL;
if((slots = unsafe_malloc(SLOTS_SIZE)))
slots[slot] = id;
return slots;
}
Did you see the difference? Its subtle. But its also easy to grep for and easy to check.
I am an advocate for checking malloc - callee-checks style. It is simply harder to screw up, and, in the overwhelming majority of cases, which you can check for yourself by randomly sampling Google Code Search, it costs you nothing in terms of reliability of functionality. Stop caller-checking malloc.
To LeBlancs other point about C++ and constructors throwing exceptions, I refer him to Cargill, or back to our blog, noting that exceptions are themselves inherently dangerous. When a language feature requires you to be that-language-feature-safe, I believe I said, you have a security problem.
As for his specific example: you cant blindly throw exceptions from a ctor. Even Meyers (MECv1, Item 9) catches that one. DONT DATE ROBOTS!
Comments
