strncpy copying more than the specified size

Question

I have the following sample code that mimics the code in the application.

#include <iostream>
#include <string.h>
#include <cstring>
#include <atlstr.h>
using namespace std;

    void test(char *s, int size)
    {
        //s = "";
        int lens = strlen(s);
        char *str1 = "(( State:0.000000 Std30c5  =  State:T ) OR (( State:0.000000 Std30c6  =  State:T )  OR (( State:0.000000 Std30c7  =  State:T ) OR (( State:0.000000 Std30c8  =  State:T ) OR (( State:0.000000 Std30c9  =  State:T ) OR (( State:0.000000 Std30ca  =  State:T ) OR (( State:0.000000 Std30cb  =  State:T ) OR (( State:0.000000 Std30cc  =  State:T ) OR (( State:0.000000 Std30cd  =  State:T ) OR (( State:0.000000 Std30ce  =  State:T ) OR (( State:0.000000 Std30cf  =  State:T ) OR ( ...0.000000   =  State:T ))))))))))))";
        int len1 = strlen(str1);
        strncpy(s, str1, 512);
        int len = strlen(s);

    }

    int main()
    {

        char strDisplay[512] = "";
        test(strDisplay, 512);


        cout << strDisplay << endl;
        system("pause");
        return 0;
    }

Result is : lenofstrtest = 523; lenofstr1 = 512;

strtest = "(( State:0.000000 Std30c5 = State:T ) OR (( State:0.000000 Std30c6 = State:T ) OR (( State:0.000000 Std30c7 = State:T ) OR (( State:0.000000 Std30c8 = State:T ) OR (( State:0.000000 Std30c9 = State:T ) OR (( State:0.000000 Std30ca = State:T ) OR (( State:0.000000 Std30cb = State:T ) OR (( State:0.000000 Std30cc = State:T ) OR (( State:0.000000 Std30cd = State:T ) OR (( State:0.000000 Std30ce = State:T ) OR (( State:0.000000 Std30cf = State:T ) OR ( ...0.000000 = State:T ))))))))))))ÌÌÌÌJ¢Š£øø)"

Why is strncpy copying additional characters?

(This is causing an issue since the incorrect strnlen is causing the unpacking logic to go haywire!)

I guess this is related to "strncpy bug 512 bytes"...please help me understand this bug.

Mechanotherapy

int len = strlen(strDisplay); cout << len <<endl; its 512 not 523 – Alterative

Answer 1

strncpy doesn't add a terminating the '\0' character to truncated strings which causes problems like you experienced. When the string is not terminated correctly then it looks like it is longer but what you are actually seeing is the data that is placed after your buffer in memory. It can cause to serious problems.

Instead of strncpy you should use strlcpy which does terminate the string correctly and returns the length of the source string which you can compare to the length of your buffer to know if the string was truncated or not. strncpy returns the pointer to the pointer to your buffer (which is not very useful since you already know it - you passed it as the first argument) and doesn't tell you whether any truncation has taken place.

See man strlcpy:

The strlcpy() and strlcat() functions copy and concatenate strings with the same input parameters and output result as snprintf(3). They are designed to be safer, more consistent, and less error prone replacements for the easily misused functions strncpy(3) and strncat(3). strlcpy() and strlcat() take the full size of the destination buffer and guarantee NUL-termination if there is room. Note that room for the NUL should be included in dstsize.

and C string handling - Replacements on Wikipedia:

The most popular[a] replacement are the strlcat and strlcpy functions, which appeared in OpenBSD 2.4 in December, 1998.[84] These functions always write one NUL to the destination buffer, truncating the result if necessary, and return the size of buffer that would be needed, which allows detection of the truncation and provides a size for creating a new buffer that will not truncate.

Unfortunately it is not included in glibc - see the Secure Portability paper by Damien Miller (PDF):

The strlcpy and strlcat API properly check the target buffer’s bounds, nul-terminate in all cases and return the length of the source string, allowing detection of truncation. This API has been adopted by most modern operating systems and many standalone software packages, including OpenBSD (where it originated), Sun Solaris, FreeBSD, NetBSD, the Linux kernel, rsync and the GNOME project. The notable exception is the GNU standard C library, glibc [12], whose maintainer steadfastly refuses to include these improved APIs, labelling them “horribly inefficient BSD crap” [4], despite prior evidence that they are faster is most cases than the APIs they replace [13]. As a result, over 100 of the software packages present in the OpenBSD ports tree maintain their own strlcpy and/or strlcat replacements or equivalent APIs - not an ideal state of affairs.

It is available for Linux in the libbsd library:

• https://libbsd.freedesktop.org/wiki/

There are packages in Debian and Ubuntu and other distros:

• https://packages.debian.org/sid/libbsd-dev
• http://packages.ubuntu.com/search?keywords=libbsd-dev

Even if you don't want to depend on anything other than glibc it is very easy to add to your project since the entire source is short ana available under a permissive license:

/*
 * Copyright (c) 1998 Todd C. Miller <[email protected]>
 *
 * Permission to use, copy, modify, and distribute this software for any
 * purpose with or without fee is hereby granted, provided that the above
 * copyright notice and this permission notice appear in all copies.
 *
 * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
 * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
 * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
 * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
 * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
 * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 */

#include <sys/types.h>
#include <string.h>

/*
 * Copy src to string dst of size siz.  At most siz-1 characters
 * will be copied.  Always NUL terminates (unless siz == 0).
 * Returns strlen(src); if retval >= siz, truncation occurred.
 */
size_t
strlcpy(char *dst, const char *src, size_t siz)
{
    char *d = dst;
    const char *s = src;
    size_t n = siz;

    /* Copy as many bytes as will fit */
    if (n != 0) {
        while (--n != 0) {
            if ((*d++ = *s++) == '\0')
                break;
        }
    }

    /* Not enough room in dst, add NUL and traverse rest of src */
    if (n == 0) {
        if (siz != 0)
            *d = '\0';      /* NUL-terminate dst */
        while (*s++)
            ;
    }

    return(s - src - 1);    /* count does not include NUL */
}

Source: http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/lib/libc/string/strlcpy.c?rev=1.11

Sod

Answer 2

When you use function strncpy then you always must append it with terminating zero. For example

strncpy(s, str1, n );
s[n-1] = '\0';

Otherwise the code will be unsafe as in your case.

Take into account that there is no any sense to use these two headers together

#include <string.h>
#include <cstring>

Remove the first header and use only the second header in C++.

#include <cstring>

Sickening

Answer 3

strDisplay should have at least 513 units assigned since no null terminating character is implicitly added within strncpy.

char strDisplay[513] = "";
strDisplay[512] = '\0'; //recommended

Pyroxenite

...and you need to actually put the null terminator in. – Anecdotal

Alternatively, he could simply code his program to eliminate these errors altogether. – Bunyabunya

Answer 4

Use std::string in C++. It will automatically take care of all of these problems for you.

#include <iostream>
#include <string>
std::string test()
{
    return "(( State:0.000000 Std30c5  =  State:T ) OR (( State:0.000000 Std30c6  =  State:T )  OR (( State:0.000000 Std30c7  =  State:T ) OR (( State:0.000000 Std30c8  =  State:T ) OR (( State:0.000000 Std30c9  =  State:T ) OR (( State:0.000000 Std30ca  =  State:T ) OR (( State:0.000000 Std30cb  =  State:T ) OR (( State:0.000000 Std30cc  =  State:T ) OR (( State:0.000000 Std30cd  =  State:T ) OR (( State:0.000000 Std30ce  =  State:T ) OR (( State:0.000000 Std30cf  =  State:T ) OR ( ...0.000000   =  State:T ))))))))))))";
}

int main()
{        
    std::cout << test() << std::endl;
    return 0;
}

Observe that no memory management, temporary magic-size buffers, or null terminators are required.

Bunyabunya

Answer 5

strncpy is a bad function because it doesn't generate a string. If you want to use C-style string handling then snprintf is easier to use safely:

snprintf(s, size, "%s", str1);

Note that char *str1 = "... is deprecated in C++; you can use char const *str1 = "... instead.

Anecdotal