How to prevent memcpy buffer overflow?

Question

There are some binary buffers with fixed sizes in a program that are used to store data, and memcpy is used to copy the buffer from one to another one. Since the source buffer may be larger than the destination buffer, how can I detect if there is buffer overflow?

Answer 1

You have to know how much data is in the source buffer and how much space is available in the target buffer.

Do not call memcpy() if there is not enough space in the target buffer for all the data you want to copy from the source buffer. (You have to decide whether it is OK to truncate the data if the source is bigger than the target.)

If you don't know, rewrite the code so that you do know how much space there is; otherwise, it is not safe.

Note that if there is a chance of the source and target buffers overlapping, you should use memmove() rather then memcpy().

In C++, look askance at using memcpy() in the first place; that is a C-style operation rather than C++.

Answer 2

How can I detect if there is buffer overflow?

I think you have three or four choices (give or take).

---

The first choice is to provide a "safe" function for memcpy. This is what I require in code under my purview, and I regularly audit for it. I also require all parameters are validated, and all parameters are asserted.

The assertions create self debugging code. I want developers to write code; and I don't want them to waste time debugging. So I require them to write code that debugs itself. ASSERTs also documents things rather well, so they can skimp on the documentation. In release builds, the ASSERTs are removed by preporcessor macros.

errno_t safe_memcpy(void* dest, size_t dsize, void* src, size_t ssize, size_t cnt)
{
    ASSERT(dest != NULL);
    ASSERT(src != NULL);
    ASSERT(dsize != 0);
    ASSERT(ssize != 0);
    ASSERT(cnt != 0);

    // What was the point of this call?
    if(cnt == 0)
        retrn 0;

    if(dest == NULL || src == NULL)
        return EINVALID;

    if(dsize == 0 || ssize == 0)
        return EINVALID;

    ASSERT(dsize <= RSIZE_MAX);
    ASSERT(ssize <= RSIZE_MAX);
    ASSERT(cnt <= RSIZE_MAX);

    if(dsize > RSIZE_MAX || ssize > RSIZE_MAX || cnt > RSIZE_MAX)
        return EINVALID;

    size_t cc = min(min(dsize, ssize), cnt);
    memmove(dest, src, cc);

    if(cc != cnt)
        return ETRUNCATE;

    return 0;
}

If your safe_memcpy returns non-0, then there was an error like a bad parameter or potential buffer overflow.

---

The second choice is to use "safer" functions provided by the C Standard. C has "safer" functions via ISO/IEC TR 24731-1, Bounds Checking Interfaces. On conforming platforms, you can simply call gets_s and sprintf_s. They offer consistent behavior (like always ensuring a string is NULL terminated) and consistent return values (like 0 on success or an errno_t).

errno_t  err = memcpy_s(dest, dsize, src, cnt);
...

Unfortunately, gcc and glibc does not conform to the C Standard. Ulrich Drepper (one of the glibc maintainers) called bounds checking interfaces "horribly inefficient BSD crap", and they were never added.

---

The third choice is to use the platform's "safer" interfaces, if present. On Windows, that happens to be the same as those in ISO/IEC TR 24731-1, Bounds Checking Interfaces. You also have the String Safe library.

On Apple and BSD, you have don't have a "safer" function for memcpy. But you do have safer string functions like strlcpy, strlcat and friends.

---

On Linux, your fourth choice is to use FORTIFY_SOURCE. FORTIFY_SOURCE uses "safer" variants of high risk functions like memcpy, strcpy and gets. The compiler uses the safer variants when it can deduce the destination buffer size. If the copy would exceed the destination buffer size, then the program calls abort(). If the compiler cannot deduce the destination buffer size, then the "safer" variants are not used.

To disable FORTIFY_SOURCE for testing, you should compile the program with -U_FORTIFY_SOURCE or -D_FORTIFY_SOURCE=0.