Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

sprintf_s with a buffer too small

The following code causes an error and kills my application. It makes sense as the buffer is only 10 bytes long and the text is 22 bytes long (buffer overflow).

char buffer[10];    
int length = sprintf_s( buffer, 10, "1234567890.1234567890." ); 

How do I catch this error so I can report it instead of crashing my application?

Edit:

After reading the comments below I went with _snprintf_s. If it returns a -1 value then the buffer was not updated.

length = _snprintf_s( buffer, 10, 9, "123456789" );
printf( "1) Length=%d\n", length ); // Length == 9

length = _snprintf_s( buffer, 10, 9, "1234567890.1234567890." );
printf( "2) Length=%d\n", length ); // Length == -1

length = _snprintf_s( buffer, 10, 10, "1234567890.1234567890." );
printf( "3) Length=%d\n", length ); // Crash, it needs room for the NULL char 
like image 861
Steven Smethurst Avatar asked Oct 01 '09 19:10

Steven Smethurst


3 Answers

Instead of sprintf_s, you could use snprintf (a.k.a _snprintf on windows).

#ifdef WIN32
#define snprintf _snprintf
#endif

char buffer[10];    
int length = snprintf( buffer, 10, "1234567890.1234567890." );
// unix snprintf returns length output would actually require;
// windows _snprintf returns actual output length if output fits, else negative
if (length >= sizeof(buffer) || length<0) 
{
    /* error handling */
}
like image 79
Managu Avatar answered Oct 22 '22 02:10

Managu


It's by design. The entire point of sprintf_s, and other functions from the *_s family, is to catch buffer overrun errors and treat them as precondition violations. This means that they're not really meant to be recoverable. This is designed to catch errors only - you shouldn't ever call sprintf_s if you know the string can be too large for a destination buffer. In that case, use strlen first to check and decide whether you need to trim.

like image 45
Pavel Minaev Avatar answered Oct 22 '22 01:10

Pavel Minaev


This works with VC++ and is even safer than using snprintf (and certainly safer than _snprintf):

void TestString(const char* pEvil)
{
  char buffer[100];
  _snprintf_s(buffer, _TRUNCATE, "Some data: %s\n", pEvil);
}

The _TRUNCATE flag indicates that the string should be truncated. In this form the size of the buffer isn't actually passed in, which (paradoxically!) is what makes it so safe. The compiler uses template magic to infer the buffer size which means it cannot be incorrectly specified (a surprisingly common error). This technique can be applied to create other safe string wrappers, as described in my blog post here: https://randomascii.wordpress.com/2013/04/03/stop-using-strncpy-already/

like image 40
Bruce Dawson Avatar answered Oct 22 '22 03:10

Bruce Dawson