Writing unsafe code with strncpy

Buffer overflow is a common source of security vulnerabilities. Just look at the CERT list for current issues.

In C and C++, a common source of buffer overflow is the string manipulations functions like strcat, sprintf and strcpy.

In an effort to write secure code with strcpy, I have seen code like this, using strncpy to prevent the buffer overflow:

In a header file somewhere:

   const int MAX_NAME_SIZE = 100;

   struct something

   {

      char name[MAX_NAME_SIZE+1];

   };

In some other place:

   ...

   something s;

   strncpy( s.name, source, MAX_NAME_SIZE+1 );

   s.name[MAX_NAME_SIZE] = '\0';

This is bad code.

It looks safe, but it is not.

Some day someone will make this change for a perfectly valid reason:

   const int NEW_MAX_NAME_SIZE = 10;

   struct something

   {

      char name[NEW_MAX_NAME_SIZE+1];

   };

If we are lucky, the old constant is deleted, causing a compiler error that forces a review of the code.

If the old constant is left around, we now have a very big bug in the code: despite the use of strncpy, there is a buffer overflow in this code.

The easy way to fix it is to refer to let the compiler calculated the size of the destination field:

   ...

   something s;

   strncpy( s.name, source, sizeof s.name );

   s.name[(sizeof s.name)-1] = '\0';