10

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 
Steven Smethurst
  • 4,495
  • 15
  • 55
  • 92
  • Passing the buffer size and the buffer size minus one is obtuse and error prone. You should prefer the variant I describe below: length = _snprintf_s(buffer, _TRUNCATE, "1234567890.1234567890." ); Since the first size parameter is omitted the compiler uses the template overload which infers the size. _TRUNCATE is a special value that does what it says. No magic numbers, and now your code is safe, maintainable, and a good example. If you like this comment and _snprintf_s then you should select my answer, instead of the dangerous snprintf/_snprintf answer. – Bruce Dawson Dec 17 '14 at 22:40

6 Answers6

17

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.

Pavel Minaev
  • 99,783
  • 25
  • 219
  • 289
  • I disagree. It is entirely reasonable to call sprintf_s with a destination buffer that is too small, as long as you use the _TRUNCATE flag to indicate that. Okay, technically _TRUNCATE requires using snprintf_s instead of sprintf_s, but my point mostly stands. Using strlen is often inapplicable or inconvenient, but using _TRUNCATE is often trivial and appropriate. – Bruce Dawson Nov 16 '15 at 23:05
  • I think that using `snprintf_s` is the crucial difference, and not really a mere technicality. – Pavel Minaev Nov 17 '15 at 02:52
  • It is a crucial difference, definitely. But I think your answer makes it seem like the _s family of functions can't truncate, which could be misleading. – Bruce Dawson Nov 18 '15 at 17:05
  • Yes you should never make any coding mistake, and then there would be no need for exceptions. It's not obvious but you need to override the default error handler for the c runtime functions then you can handle them as you please, including throwing your own exception. See _set_invalid_parameter_handler [link](https://msdn.microsoft.com/en-us/library/a9yf33zb.aspx) and Jonathon Leffler's answer. – GBrookman Feb 02 '17 at 16:25
8

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 */
}
Managu
  • 8,849
  • 2
  • 30
  • 36
  • 2
    Note: as a matter of security, if there's not enough room, the contents of buffer might not be null terminated. – Managu Oct 01 '09 at 20:12
  • 2
    @Managu: if MS claimed conformance to C99 - which it doesn't - that assertion would be bogus; the C99 standard requires snprintf() to null terminate the string unless the length of the string is 0. Section 7.19.6.5: If n is zero, nothing is written... Otherwise, output characters beyond the n-1st are discarded rather than being written to the array, and a null character is written at the end of the characters actually written into the array. If copying takes place between objects that overlap, the behavior is undefined. – Jonathan Leffler Oct 01 '09 at 22:24
  • 2
    @Managu Please edit your answer to remove the suggestion to use _snprintf. This is an incredibly dangerous suggestion. – Bruce Dawson Nov 22 '14 at 00:42
  • VC++ 2015 supports snprintf, so that can now be used. Do not use _snprintf as that is different. But for true safety (to avoid the problems of incorrectly specifying the buffer size) us sprintf_s or snprintf_s (see answers below). – Bruce Dawson Nov 16 '15 at 23:07
8

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/

Bruce Dawson
  • 3,284
  • 29
  • 38
  • looking at the MSDN documentation of _snprintf_s, it appears you have forgotten an argument in the call to _snprintf_s. This argument should appear between buffer and _TRUNCATE and is called sizeOfBuffer – user1741137 Oct 22 '17 at 14:02
  • 2
    I have not forgotten an argument - that code compiles and is perfectly safe. You need to re-read the documentation. I am making use of a template override to _snprintf_s that tells the compiler to infer the buffer size. I have seen hundreds of places where programmers have explicitly passed in a buffer size and have passed in the *wrong* size. Only by having the compiler infer the buffer size can this serious bug be avoided. I alluded to this technique in the article that I linked to in my solution. Strongly recommended. – Bruce Dawson Oct 23 '17 at 16:07
0

From MSDN:

The other main difference between sprintf_s and sprintf is that sprintf_s takes a length parameter specifying the size of the output buffer in characters. If the buffer is too small for the text being printed then the buffer is set to an empty string and the invalid parameter handler is invoked. Unlike snprintf, sprintf_s guarantees that the buffer will be null-terminated (unless the buffer size is zero).

So ideally what you've written should work correctly.

Ashwin
  • 3,609
  • 2
  • 18
  • 11
0

Looks like you're writing on MSVC of some sort?

I think the MSDN docs for sprintf_s says that it assert dies, so I'm not too sure if you can programmatically catch that.

As LBushkin suggested, you're much better off using classes that manage the strings.

Calyth
  • 1,673
  • 3
  • 16
  • 26
0

See section 6.6.1 of TR24731 which is the ISO C Committee version of the functionality implemented by Microsoft. It provides functions set_constraint_handler(), abort_constraint_handler() and ignore_constraint_handler() functions.

There are comments from Pavel Minaev suggesting that the Microsoft implementation does not adhere to the TR24731 proposal (which is a 'Type 2 Tech Report'), so you may not be able to intervene, or you may have to do something different from what the TR indicates should be done. For that, scrutinize MSDN.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
  • 1
    Unfortunately, MSVC does not implement TR24731 fully - in particular, it does not implement specifically the functions that you reference (also, their names also end with `_s` - i.e. `set_constraint_handler_s`). – Pavel Minaev Oct 01 '09 at 20:04
  • 1
    But according to http://msdn.microsoft.com/en-us/library/ksazx244%28VS.80%29.aspx there is a function _set_invalid_parameter_handler() function that can be used to change the default behaviour of aborting the program. – Jonathan Leffler Oct 02 '09 at 02:26