Peeter Joot's (OLD) Blog.

Math, physics, perl, and programming obscurity.

ease of getting strncat wrong.

Posted by peeterjoot on December 5, 2012

I’d done a test build with the clang compiler of our db2 code and fired off some defects and emails to some of the code owners about messages from that compiler like:

foo.C:1220:15: warning: the value of the size argument in ‘strncat’ is too large, might lead to a buffer overflow [-Wstrncat-size]

It’s a great message, clear and unambiguous: “your code is wrong”!

Here’s a sample of the code in question that clang was complaining about:

   strncat( dirPath, ptrToDirectories, ( sizeof( dirPath ) - strlen( dirPath ) ) );

The compiler could just as well complain about simpler strncat calls.  Imagine at first that dirPath was all zeros, then the above code would be equivalent to:

   strncat( dirPath, ptrToDirectories, sizeof( dirPath ) ) ;

It’s not immediately obvious reading the strncat man page that this is wrong, since that man page says “[strncat] will use at most n characters from src”. However, it also says “As with strcat(), the resulting string in dest is always null terminated.”

Does this mean that it will use no more than n characters from src, but that it null terminates within the range of those n-characters, or does this mean use (if strlen(src) > n) n characters from src and then add a null terminator after that?

To answer that question, and rule out a possible false positive warning from the compiler I wrote the following simple bit of test code:

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

struct string
   char dest[2] ;
   char overflow ;
} ;

int main()
   string o ;
   o.dest[0] = 0  ;
   o.dest[1] = 1  ;
   o.overflow = 1 ;

   printf( "%d\n", (int)o.overflow ) ;

   strncat( o.dest, "abc", sizeof(o.dest) ) ;

   printf( "%d\n", (int)o.overflow ) ;

   return 0 ;

And the verdict was:

$ g++ -g t.C ; a.out

Yup, the code was bad, and the compiler is without fault. We actually have a different function for strncat’ing in our code that takes the actual buffer size, to make this harder to get wrong, but the code in question was not using that.

Leave a Reply

Fill in your details below or click an icon to log in: Logo

You are commenting using your account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: