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?

Solution 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++.

Solution 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.

Solution 3

You should always know and check the src and dest buffers size !

void *memcpy(void *dest, const void *src, size_t n);

n should never be greater than src or dest size.

Solution 4

If for example you have:

destination 4 bytes size

source 5 bytes size

You can make sure to copy, at most, 4 bytes to destination buffer:

size_t getCopySize(size_t sourceSize, size_t destSize)
{
    return (destSize <= sourceSize ? destSize : sourceSize);
}
memcpy(destination, source, getCopySize(sizeof(source),sizeof(destination)));

Basing on your application you could also make sure that the remaining data will be copied at a later time, or you can skip it if some data can be ignored.

View more solutions

Comments

[BSen]

Detect? You do know destination buffer size? Then write code like this memcpy(src, dst, sizeof(dst))

[SingerOfTheFall]

Compare the size of the source buffer and the destination buffer and see which is larger?

[juanchopanza]

@BSen that sizeof will just give the size of a pointer.

[BSen]

Treat it as pseudocode. It can give size of a buffer depending on declaration style. I do realize, that sizeof(dst *) gives pointer size and sizeof(dst [123]) gives buffer size, at least when 1 element = 1 byte.

[James Kanze]

@juanchopanza if dst is a pointer. If it's something else (e.g. an array) it will give the size of that.

[juanchopanza]

@JamesKanze I was assuming, maybe incorrectly, that OP was dealing with pointers, otherwise there would be no problem in the first place.

[James Kanze]

@juanchopanza IIUC, his problem is in the destination, not the source. It seems reasonable to worry about whether the data fits into the destination even if the source is an array. (None of which really helps him, since even if the source is an array, sizeof will give the size of the complete array, even if only part of it is filled.)

[Michael D]

Thanks. what is the proper way to do the memory copy in C++?

[MSalters]

@MichaelD: Store your data in a std::vector<>, and just use vector2 = vector1.

[Michael D]

How can I insert data into vector? use push_back to insert date byte by byte?

[Steve Jessop]

@MichaelD: vector has a member function named insert.

[Michael D]

If the binary data is from network e.g. tcp, how to insert to vector without sacrificing much performance?

[Steve Jessop]

@MichaelD: if you want to avoid the extra copy performed by insert, then you can pass a pointer to an element of a vector as the buffer to your socket recv function. You do have to make sure that the size of the vector is big enough first. resize on a vector will clear the bytes, so there's still a small overhead compared with writing into an uninitialized array.

[Gerd]

Unfortunately, it seems you cannot use that straight away as a nested function - you will get an 'implicit declaration' error.