Why is strncpy insecure?

Question

I am looking to find out why strncpy is considered insecure. Does anybody have any sort of documentation on this or examples of an exploit using it?

Answer 1

Take a look at this site; it's a fairly detailed explanation. Basically, strncpy() doesn't require NUL termination, and is therefore susceptible to a variety of exploits.

Answer 2

Get the book Windows Via C/C++ by Jeffrey Richter, it has a very good explanation on this and other insecure methods.

Answer 3

Take a look at strlcpy and friends (warning - PostScript.)

Answer 4

The following code explains the difference between strcpy and strncpy :-

#include <stdio.h>
#include <string.h>

#define SIZE    40

int main(void)
{
  char source[ SIZE ] = "123456789";
  char source1[ SIZE ] = "123456789";
  char destination[ SIZE ] = "abcdefg";
  char destination1[ SIZE ] = "abcdefg";
  char * return_string;
  int    index = 5;

  /* This is how strcpy works */
  printf( "destination is originally = '%s'\n", destination );
  return_string = strcpy( destination, source );
  printf( "After strcpy, destination becomes '%s'\n\n", destination );


  /* This is how strncpy works */
  printf( "destination1 is originally = '%s'\n", destination1 );
  return_string = strncpy( destination1, source1, index );
  printf( "After strncpy, destination1 becomes '%s'\n", destination1 );
}

/*****************  Output should be similar to:  *****************

destination is originally = 'abcdefg'
After strcpy, destination becomes '123456789'

destination1 is originally = 'abcdefg'
After strncpy, destination1 becomes '12345fg'
*/

So as you can see that strncpy() function copies count characters of string2 to string1. If count is less than or equal to the length of string2, a null character (\0) is not appended to the copied string. If count is greater than the length of string2, the string1 result is padded with null characters (\0) up to length count.

While The strcpy() function operates on null-ended strings. The string arguments to the function should contain a null character (\0) that marks the end of the string. No length checking is performed. You should not use a literal string for a string1 value, although string2 may be a literal string.

The following are some of the code samples displaying the exploits of strncpy() as well as strcpy() :-

1. Buffer overflow using a custom version of the strcpy() function

   char *stringcopy(char *str1, char *str2)
   {
      while (*str2)
      *str1++ = *str2++;
      return str2;
   }
   main(int argc, char **argv)
   {
      char *buffer = (char *)malloc(16 * sizeof(char));
      stringcopy(buffer, argv[1]);
      printf("%s\n", buffer);
   }
   

2. Here, the developer is getting a pathname as an argument and wants to find the first path component. The error is that the path in str might start with a '/', in which case len is zero and len-1 is the largest value possible for a size_t. In that particular case the strncpy in the else clause is no safer than a strcpy.

   #include <stdlib.h>
   void func(char *str)
   {
      char buf[1024];
      size_t len;
      char *firstslash = strchr(str, '/');
      if (!firstslash)
          strncpy(buf, str, 1023);  /* leave room for the zero */
          buf[1023] = 0;
      else
      {
          len = firstslash - str;     /* length of the first path component */
          if (len > 1023)
                len = 1023;
          strncpy(buf, str, len-1);   /* cut the slash off. Only copy len-1
         characters to avoid zero padding.  */
          buf[len] = 0;
      }
   }
   

3. This program contains a buffer overflow, but the overflowing data isn't controlled by the attacker. Ideally, a scanner should either not report a buffer overflow associated with this strcpy, or at most report a problem with lower severity than a strcpy whose argument is attacker- controlled.

   main()
   {
       char *buffer = (char *)malloc(10 * sizeof(char));
       strcpy(buffer, "fooooooooooooooooooooooooooooooooooooooooooooooooooo");
   }
   

4. strncpy doesn't automatically null-terminate the string being copied into. In this example, the attacker supplies an argv[1] of length ten or more. In the subsequent strncat, data is copied not to buffer[10] as the code suggests, but to the first location to the left of buffer[0] that happens to contain a zero byte.

    main(int argc, char **argv)
    {
        char *buffer = (char *)malloc(101);
        strncpy(buffer, argv[1], 10);
        strncat(buffer, argv[2], 90);
    }
   

5. In this example, the attacker controls the third argument of strncpy, making it unsafe.

    #include <stdio.h>
    main(int argc, char **argv)
    {
        int incorrectSize = atoi(argv[1]);
        int correctSize = atoi(argv[2]);
        char *buffer = (char *)malloc(correctSize+1);
        /* number of characters copied is based on user-supplied value */
        strncpy(buffer, argv[3], incorrectSize);
    }
   

6. The principle here is that incorrectly casting a pointer to a C++ object potentially breaks the abstraction represented by that object, since the (non-virtual) methods called on that object are determined at compile-time, while the actual type of the object might not be known until runtime. In this example, a seemingly safe strncpy causes a buffer overflow. (In gcc the buffer overflows into object itself and then onto the stack, for this particular program. With some compilers the overflow might modify the object's virtual table.) It's hard to say what a scanner should flag in this test file. In my opinion the only casts allowed should be virtual member functions that cast the this pointer to the class that owns them (e.g., As() functions) and I think that prevents this type of vulnerability.

    #include <stdio.h>
    #include <string.h>
    class Stringg 
    {};
    class LongString: public Stringg
    {
         private:
             static const int maxLength = 1023;
             char contents[1024];
         public:
             void AddString(char *str)
             {
                    strncpy(contents, str, maxLength);
                    contents[strlen(contents)] = 0;
             }
     };
     class ShortString: public Stringg
     {
         private:
         static const int maxLength = 5;
         char contents[6];
         public:
             void AddString(char *str)
             {
                   strncpy(contents, str, maxLength);
                   contents[strlen(contents)] = 0;
             }
     };
     void func(Stringg *str)
     {
          LongString *lstr = (LongString *)str;
          lstr->AddString("hello world");
     }
     main(int argc, char **argv)
     {
          ShortString str;
          func(&str);
     }
   

7. In this program, the target string is properly terminated but the terminating null is added before the strncpy(), which might fool a scanner into thinking that the buffer is unterminated.

   The scanner should not complain about an unterminated strncpy().

    void func(char *str)
    {
         char target[(strlen(str) + 1) * sizeof(char)];
         target[strlen(str)] = 0;
         strncpy(target, str, strlen(str));
    }
   

Answer 5

To safely use strncpy, one must either (1) manually stick a null character onto the result buffer, (2) know that the buffer ends with a null beforehand, and pass (length-1) to strncpy, or (3) know that the buffer will never be copied using any method that won't bound its length to the buffer length.

It's important to note that strncpy will zero-fill everything in the buffer past the copied string, while other length-limited strcpy variants will not. This may at some cases be a performance drain, but in other cases be a security advantage. For example, if one used strlcpy to copy "supercalifragilisticexpalidocious" into a buffer and then to copy "it", the buffer would hold "it^ercalifragilisticexpalidocious^" (using "^" to represent a zero byte). If the buffer gets copied to a fixed-sized format, the extra data might tag along with it.