Thread: Advice on preventing buffer overflows

Code:

char *buffs_r_us(void)
{
    char *buff = malloc(BUFFER_SIZE);
    if (buff)
    {
        /* stuff buff */
    }
    return buff;
}

This puts the onus on the function user to test for NULL and to free the buff.

Originally Posted by foniks

Is there a bullet proof way of detecting that the user has provided a buffer that is too small?

No.

The only two options are for the caller to supply the buffer by some means, or for your function to dynamically allocate a buffer and return that.

If the user supplies the buffer, there is no way to detect its length (nor to check that the user has given correct information about its length). That is the user's responsibility.

If your program allocates the buffer and returns that to the caller, then (unless you make a programming error) the buffer will be sufficient. However, the result of doing that is a memory leak, unless the user remembers to deallocate the buffer. If there are enough memory leaks in a program, a typical result is program slow-down or memory exhaustion.

Either way, you are vulnerable if you make an error of judgement or coding, or if the user does not do things correctly.

Originally Posted by foniks

That's a good point - the responsibilities the callers, but at least it's transparent (i.e. you know it returns allocated memory, you have to test for null and free the memory as well).

How is requiring a caller to test for NULL and release memory (each of which may not be done) any more transparent than requiring the caller to provide a buffer of sufficient length (which may also not be done)?

Both are responsibilities of the caller that the caller is not guaranteed to meet.

I should have began my "answer" with "NO" as grumpy did. There's no way to protect against errors here. You just get to chose from different sets of possible errors.

The user must read and comply with the documentation for the function, providing what it needs and testing return values (and possibly other values) as required. So neither method (allocating or preallocating) is perfect.

It's often best to get the user to allocate the buffer so that the allocation/deallocation is "bracketed" in the his own code (perhaps he's less likely to forget to free it if he allocates it himself), and in particular it allows the use of a local variable, automating memory management.

You might define a constant for the required size. An example of this is BUFSIZ, defined in the standard library (stdio.h) for I/O buffers.

Originally Posted by foniks

Hi,

Just wondering if someone can give me some advice on preventing a buffer overflow when dealing with strings being passed as arguments.

If I have a function prototype such as:

Code:

void foobar(char *bar);

That argument bar - is intended to take a pointer to a buffer of x characters in length. Inside that function, I can't get the size of that buffer, as bar is now just a pointer to a char. I COULD just make the user of this function pass a length parameter, but there is no guarantee that would be correct. Is there a bullet proof way of detecting that the user has provided a buffer that is too small?

Thanks.

Forcing the user to supply a length parameter for output buffers will virtually eliminate the possibility of overflows (there are still a few corner cases, but these are generally caught elsewhere in the program and easily identified). Personally, I wouldn't use any library function that doesn't require buffer lengths for output - besides posing a security threat to anyone relying on my program, it's just too damn easy to forget to allocate enough memory.

As for input parameters, it's often sufficient to expect a null-terminated string (depends on the data format, of course); the data is effectively read-only so no possibility for overflow anyway...

Originally Posted by foniks

@grumpy - because as a user of that function it's not clear what the function may be doing, what checks it's doing etc. But in the other case, I know I have to do something with that memory. I may not do it - and that'd be wrong, but the onus is now on me. Not on someone else code that I may not be able to see.

Sorry, but I think your premise is flawed.

The caller of a function does not need to know what checks the function does. All it needs to know is what that function expects of a caller - in terms of what the caller does BEFORE calling your function (say, allocating a buffer of required length) or what the caller is required to do AFTER calling your function (say, deallocating a buffer that your function has allocated).

Either way, you're relying on correct behaviour of code you may not be able to see.