98

strncpy() supposedly protects from buffer overflows. But if it prevents an overflow without null terminating, in all likelihood a subsequent string operation is going to overflow. So to protect against this I find myself doing:

strncpy( dest, src, LEN );
dest[LEN - 1] = '\0';

man strncpy gives:

The strncpy() function is similar, except that not more than n bytes of src are copied. Thus, if there is no null byte among the first n bytes of src, the result will not be null-terminated.

Without null terminating something seemingly innocent like:

   printf( "FOO: %s\n", dest );

...could crash.


Are there better, safer alternatives to strncpy()?

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
Timothy Pratley
  • 10,586
  • 3
  • 34
  • 63
  • 1
    Note that on MacOS X (BSD) the man page says (of '`extern char *strncpy(char * restrict s1, const char * restrict s2, size_t n);`'): The strncpy() function copies at most n characters from s2 into s1. If s2 is less than n characters long, the remainder of s1 is filled with `\0' characters. Otherwise, s1 is not terminated. – Jonathan Leffler Sep 21 '09 at 21:31
  • Shouldnt it be dest[LEN-1] = '\0'; ? – codeObserver May 18 '11 at 17:29
  • 2
    This is how I would think we would make a copy of the string:int LEN = src.len; str* dest = new char[LEN+1]; strncpy( dest, src, LEN ); dest[LEN] = '\0'; – codeObserver May 18 '11 at 17:30
  • Always use memset on destination string is the safest approach if you are sure of size of the string will not exceed the destination buffer length. – koolvcvc Jul 04 '14 at 12:02
  • write your own function, i dont think that should be a tough task – Megharaj Mar 30 '17 at 11:26

11 Answers11

58

strncpy() is not intended to be used as a safer strcpy(), it is supposed to be used to insert one string in the middle of another.

All those "safe" string handling functions such as snprintf() and vsnprintf() are fixes that have been added in later standards to mitigate buffer overflow exploits etc.

Wikipedia mentions strncat() as an alternative to writing your own safe strncpy():

*dst = '\0';
strncat(dst, src, LEN);

EDIT

I missed that strncat() exceeds LEN characters when null terminating the string if it is longer or equal to LEN char's.

Anyway, the point of using strncat() instead of any homegrown solution such as memcpy(..., strlen(...))/whatever is that the implementation of strncat() might be target/platform optimized in the library.

Of course you need to check that dst holds at least the nullchar, so the correct use of strncat() would be something like:

if (LEN) {
    *dst = '\0'; strncat(dst, src, LEN-1);
}

I also admit that strncpy() is not very useful for copying a substring into another string, if the src is shorter than n char's, the destination string will be truncated.

pevik
  • 4,523
  • 3
  • 33
  • 44
Ernelli
  • 3,960
  • 3
  • 28
  • 34
  • 34
    "it is supposed to be used to insert one string in the middle of another" - no, it's intended to write a string into a fixed-width field, such as in a directory entry. That's why it pads the output buffer with NUL if (and only if) the source string is too short. – Steve Jessop Sep 21 '09 at 11:19
  • 5
    How does setting *dst='\0' make this any safer? It still has the original problem of allowing you to write beyond the end of the target buffer. – Adam Liss Sep 21 '09 at 11:22
  • Because he's using `strncat`, which *concatenates* onto the destination string. – Martin B Sep 21 '09 at 11:27
  • `*dst = 0` is just there to make strncat work at all (i.e. write to the start of the destination buffer). If LEN+1 is the length of the target buffer, then that is what prevents overrun. Unlike strncpy, strncat always NUL-terminates the output, which is why the above code is a "safer" strcpy alternative than strncpy. Note though that the above code does write up to LEN+1 bytes, not up to LEN bytes, so there is still an excellent opportunity for the user to get it wrong. – Steve Jessop Sep 21 '09 at 11:32
  • 5
    sounds good but shouldn't it be strncat(dst,src,LEN-1) given its going to write an extra character? – Timothy Pratley Sep 21 '09 at 11:37
  • ah I guess it all depends if dst is LEN or LEN+1 long :) – Timothy Pratley Sep 21 '09 at 11:38
  • Since `strncpy()` null pads a short string to the specified length, I'm not convinced that it is intended for insertion into the middle of other strings. I'll agree with the first part of the first sentence. And the general use of `strncat()` is dangerous; the calling convention is complex if the destination is not an empty string. Granted, the example shows using `strncat()` on an empty string - because of the `*dst = '\0';` assignment. But the length has to be reduced by the actual length of the current value in the destination to be safe. The TR24731 safe functions are safer. – Jonathan Leffler Sep 21 '09 at 11:41
  • 3
    @Jonathan: Actually safe would be a datatype combining a pointer to a char buffer, with the length of that buffer. But we all know that's not going to happen. Personally, I'm bone-weary of all these efforts to make something which is inherently unsafe (programmers trying to accurately respect the length of a buffer), some fraction safer. It's not as if we currently have 50% too many buffer overruns, so if only we could make string handling 50% safer we'd be fine :-( – Steve Jessop Sep 21 '09 at 12:06
  • It's already been said, but my downvote is due to this explanation making no sense, and the suggestion is less than useful -- why not just wrap strncpy and supply the terminal null? Some implementations (as noted by Leffler above) already do specify that they will pad the output with nulls which seems to me to be the right thing to do. – c-urchin Aug 05 '10 at 08:27
  • 2
    +1 for not repeating the rubbish that strncpy is somehow a safe version of strcpy - the former has its own set of problems. – paxdiablo Feb 12 '13 at 07:28
  • Enjoy your guru badge : ) – hat Feb 04 '19 at 10:07
40

Originally, the 7th Edition UNIX file system (see DIR(5)) had directory entries that limited file names to 14 bytes; each entry in a directory consisted of 2 bytes for the inode number plus 14 bytes for the name, null padded to 14 characters, but not necessarily null-terminated. It's my belief that strncpy() was designed to work with those directory structures - or, at least, it works perfectly for that structure.

Consider:

  • A 14 character file name was not null terminated.
  • If the name was shorter than 14 bytes, it was null padded to full length (14 bytes).

This is exactly what would be achieved by:

strncpy(inode->d_name, filename, 14);

So, strncpy() was ideally fitted to its original niche application. It was only coincidentally about preventing overflows of null-terminated strings.

(Note that null padding up to the length 14 is not a serious overhead - if the length of the buffer is 4 KB and all you want is to safely copy 20 characters into it, then the extra 4075 nulls is serious overkill, and can easily lead to quadratic behaviour if you are repeatedly adding material to a long buffer.)

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
  • 4
    That particular situation may be obscure, but it's not exactly uncommon to have data structures with fixed-length string fields which are null-padded but not null-terminated. Indeed, if one is storing fixed-format data, that's oftentimes the most efficient way to do it. – supercat Nov 20 '11 at 22:54
26

There are already open source implementations like strlcpy that do safe copying.

http://en.wikipedia.org/wiki/Strlcpy

In the references there are links to the sources.

StampedeXV
  • 2,715
  • 2
  • 24
  • 40
  • 1
    Not to mention, portable, fast and reliable. You can still misuse it, but the risk is orders of magnitude lower. IMO, strncpy should be deprecated and replaced with the same function called dirnamecpy or something like that. strncpy is not safe string copy and has never been. –  Sep 21 '09 at 23:38
8

Some new alternatives are specified in ISO/IEC TR 24731 (Check https://buildsecurityin.us-cert.gov/daisy/bsi/articles/knowledge/coding/317-BSI.html for info). Most of these functions take an additional parameter that specifies the maximum length of the target variable, ensure that all strings are null-terminated, and have names that end in _s (for "safe" ?) to differentiate them from their earlier "unsafe" versions.1

Unfortunately, they're still gaining support and may not be available with your particular tool set. Later versions of Visual Studio will throw warnings if you use the old unsafe functions.

If your tools don't support the new functions, it should be fairly easy to create your own wrappers for the old functions. Here's an example:

errCode_t strncpy_safe(char *sDst, size_t lenDst,
                       const char *sSrc, size_t count)
{
    // No NULLs allowed.
    if (sDst == NULL  ||  sSrc == NULL)
        return ERR_INVALID_ARGUMENT;

   // Validate buffer space.
   if (count >= lenDst)
        return ERR_BUFFER_OVERFLOW;

   // Copy and always null-terminate
   memcpy(sDst, sSrc, count);
   *(sDst + count) = '\0';

   return OK;
}

You can change the function to suit your needs, for example, to always copy as much of the string as possible without overflowing. In fact, the VC++ implementation can do this if you pass _TRUNCATE as the count.




1Of course, you still need to be accurate about the size of the target buffer: if you supply a 3-character buffer but tell strcpy_s() it has space for 25 chars, you're still in trouble.
Adam Liss
  • 47,594
  • 12
  • 108
  • 150
  • You can't legally define a function whose name starts with str*, that "namespace" is reserved in C. – unwind Sep 21 '09 at 11:28
  • 2
    But the ISO C committee can - and did. See also: http://stackoverflow.com/questions/372980/do-you-use-the-tr-24731-safe-functions-in-your-c-code – Jonathan Leffler Sep 21 '09 at 11:42
  • @Jonathan: Thanks for the cross-reference to your own question, which provides a lot of additional helpful info. – Adam Liss Sep 21 '09 at 12:08
8

Strncpy is safer against stack overflow attacks by the user of your program, it doesn't protect you against errors you the programmer do, such as printing a non-null-terminated string, the way you've described.

You can avoid crashing from the problem you've described by limiting the number of chars printed by printf:

char my_string[10];
//other code here
printf("%.9s",my_string); //limit the number of chars to be printed to 9
Liran Orevi
  • 4,755
  • 7
  • 47
  • 64
  • The use of the precision field to limit the number of characters printed by `%s` has got to be one of the most obscure features of C. – David Thornley Dec 18 '09 at 17:59
  • @DavidThornley It is documented very clearly in K&R under sprintf. – weston Mar 15 '12 at 12:43
  • @weston: And in Harbison & Steele, which is what I've got here at work. Now, in what popular C books, other than those two, is this mentioned? Every feature should be mentioned in K&R and H&S (and is mentioned in the Standard), so if that's the standard of obscurity there are no obscure features. – David Thornley Mar 15 '12 at 13:48
  • @DavidThornley I just wanted to balance your comment, because by putting "one of the most obscure features" it makes this answer look bad and people could be put off from using it. Which is wrong as it's a perfectly valid, well documented feature, as well documented as any other use of the precision field. "Obscure" seems to be a matter of opinion, as personally I see this used a lot. – weston Mar 16 '12 at 09:22
5

Use strlcpy(), specified here: http://www.courtesan.com/todd/papers/strlcpy.html

If your libc doesn't have an implementation, then try this one:

size_t strlcpy(char* dst, const char* src, size_t bufsize)
{
  size_t srclen =strlen(src);
  size_t result =srclen; /* Result is always the length of the src string */
  if(bufsize>0)
  {
    if(srclen>=bufsize)
       srclen=bufsize-1;
    if(srclen>0)
       memcpy(dst,src,srclen);
    dst[srclen]='\0';
  }
  return result;
}

(Written by me in 2004 - dedicated to the public domain.)

alex tingle
  • 6,920
  • 3
  • 25
  • 29
  • please enlighten me, why you want the result is always the length of the src string? in my opinion, return `srclen` will be better since we would know how many characters are really copied. – Lê Quang Duy May 03 '18 at 02:17
  • @LêQuangDuy, it's in keeping with the spec (https://www.freebsd.org/cgi/man.cgi?query=strlcpy&sektion=3#end): like **snprintf**, **strlcat**, it returns the size of the string it *tried* to write, so the caller can provide a larger buffer and re-invoke the function to store everything. – Jonathan Lidbeck May 03 '19 at 23:38
5

Instead of strncpy(), you could use

snprintf(buffer, BUFFER_SIZE, "%s", src);

Here's a one-liner which copies at most size-1 non-null characters from src to dest and adds a null terminator:

static inline void cpystr(char *dest, const char *src, size_t size)
{ if(size) while((*dest++ = --size ? *src++ : 0)); }
Christoph
  • 164,997
  • 36
  • 182
  • 240
  • We're using macro equivalent to `snprintf(buffer, sizeof(buffer), "%s", src)`. Works fine as long as you remember to never use it on char * destinations – che Feb 01 '13 at 08:30
3

strncpy works directly with the string buffers available, if you are working directly with your memory, you MUST now buffer sizes and you could set the '\0' manually.

I believe there is no better alternative in plain C, but its not really that bad if you are as careful as you should be when playing with raw memory.

Arkaitz Jimenez
  • 22,500
  • 11
  • 75
  • 105
3

I have always preferred:

 memset(dest, 0, LEN);
 strncpy(dest, src, LEN - 1);

to the fix it up afterwards approach, but that is really just a matter of preference.

Delimitry
  • 2,987
  • 4
  • 30
  • 39
stonemetal
  • 6,111
  • 23
  • 25
  • 1
    Whether to initialize all buffers to zero is a topic of debate in its own right. Personally, I prefer to do so during development/debugging, as it tends to make errors more obvious, but there are plenty of other ("cheaper") options. – Adam Liss Sep 21 '09 at 12:14
  • 8
    you only need to set `dest[LEN-1]` to `0` - the other bytes will be filled by `strncpy()` if needed (remember: `strncpy(s,d,n)` ALWAYS writes `n` bytes!) – Christoph Sep 21 '09 at 13:59
3

Without relying on newer extensions, I have done something like this in the past:

/* copy N "visible" chars, adding a null in the position just beyond them */
#define MSTRNCPY( dst, src, len) ( strncpy( (dst), (src), (len)), (dst)[ (len) ] = '\0')

and perhaps even:

/* pull up to size - 1 "visible" characters into a fixed size buffer of known size */
#define MFBCPY( dst, src) MSTRNCPY( (dst), (src), sizeof( dst) - 1)

Why the macros instead of newer "built-in" (?) functions? Because there used to be quite a few different unices, as well as other non-unix (non-windows) environments that I had to port to back when I was doing C on a daily basis.

Roboprog
  • 3,054
  • 2
  • 28
  • 27
2

These functions have evolved more than being designed, so there really is no "why". You just have to learn "how". Unfortunately the linux man pages at least are devoid of common use case examples for these functions, and I've noticed lots of misuse in code I've reviewed. I've made some notes here: http://www.pixelbeat.org/programming/gcc/string_buffers.html

pixelbeat
  • 30,615
  • 9
  • 51
  • 60
  • Erm why is the _ mangled to %5F in the URL above? Underscore is fine as per RFC 3548. – pixelbeat Sep 21 '09 at 13:36
  • Given `strncpy()` as it exists, one can force the string to be zero-terminated by manually writing a zero byte at the end of the buffer. By contrast, if strncpy() insisted upon always writing a zero-byte following the last useful location, I can't think of any efficient way of updating zero-padded (not terminated) strings. Note that zero-padded strings of known fixed length were and still are a time-efficient means of storing data on disk; storing information in RAM in the same format as it is on disk can also boost performance. – supercat Nov 20 '11 at 23:04