Which of sprintf/snprintf is more secure?

Question

I wish to know which of these two options is the more secure one to use:

#define MAXLEN 255
char buff[MAXLEN + 1]

1. sprintf(buff, "%.*s", MAXLEN, name)

2. snprintf(buff, MAXLEN, "%s", name)

My understanding is that both are same. Please suggest.

Roundtree

Change #2 to MAXLEN+1 and they'll be identical in what they write into buff in all cases (the return value will be different if strlen(name) > 255). – Frederick

Answer 1

The two expressions you gave are not equivalent: sprintf takes no argument specifying the maximum number of bytes to write; it simply takes a destination buffer, a format string, and a bunch of arguments. Therefore, it may write more bytes than your buffer has space for, and in so doing write arbitrary code. The %.*s is not a satisfactory solution because:

1. When the format specifier refers to length, it's referring to the equivalent of strlen; this is a measure of the number of characters in the string, not its length in memory (i.e. it doesn't count the null terminator).
2. Any change in the format string (adding a newline, for example) will change the behavior of the sprintf version with respect to buffer overflows. With snprintf, a fixed, clear maximum is set regardless of changes in the format string or input types.

Genia

Actually, about 1, I was mistaken - it does specify the maximum number of characters (but without counting the \0). I've edited my answer accordingly. – Sharmainesharman

Thanks - I remembered there being some problem with that format specifier, and assumed you were right :-P – Genia

Well, this all depends on what kind of security the OP's question is about. Formally a properly used sprintf is as secure in this specific case as snprintf. What you are talking about in this answer is the lack of protection from lazy/incompetent programmer. Whether the OP was asking about this side of security I don't know. – Launcher

I'm assuming that the string name is provided by an untrusted user; that's the only case where this is a security issue at all, and in that case it is a rather serious one. With a really bad sprintf format string, a malicious user carefully crafting their input can overwrite the stack and execute arbitrary code; with this format string, a malicious user can only overwrite one byte on the stack with a zero (the null terminator), which might be a pretty effective DOS attack. – Genia

Still, a good format string provides the same level of protection as snprintf (again, applied to string input specifically). – Launcher

It does, but it's much less verifiable - it's hard to look at an sprintf call and, at a glance, establish an upper limit to the memory it will overwrite. – Genia

Answer 2

The best and most flexible way would be to use snprintf!

size_t nbytes = snprintf(NULL, 0, "%s", name) + 1; /* +1 for the '\0' */
char *str = malloc(nbytes);
snprintf(str, nbytes, "%s", name);

In C99, snprintf returns the number of bytes written to the string excluding the '\0'. If there were less than the necessary amount of bytes, snprintf returns the number of bytes that would have been necessary to expand the format (still excluding the '\0'). By passing snprintf a string of 0 length, you can find out ahead of time how long the expanded string would have been, and use it to allocate the necessary memory.

Jeffcott

Yes, this is the best approach, I don't know why this answer is not voted to the top. Users usually don't know that they can put NULL as the first argument. – Segregationist

man7.org/linux/man-pages/man3/asprintf.3.html takes it one step further and just allicates it for you. GNU extension. – Jaddan

Performance-wise, does this solution use about double the time? – Chenopod

Answer 3

For the simple example in the question, there might not be much difference in the security between the two calls. However, in the general case snprintf() is probably more secure. Once you have a more complex format string with multiple conversion specifications it can be difficult (or near impossible) to ensure that you have the buffer length accounted for accurately across the different conversions - especially since a previous conversions don't necessarily produce a fixed number of output characters.

So, I'd stick with snprintf().

Another small advantage to snprintf() (though not security related) is that it'll tell you how big of a buffer you need.

A final note - you should specify the actual buffer size in the snprintf() call - it'll handle accounting for the null terminator for you:

snprintf(buff, sizeof(buff), "%s", name);

Othella

I think it's worth stating explicitly that for your last line of code, buff has to be an array on the stack. If you have a char *, it won't work. (Well, it might, but it won't work all the time, at least.) Yes, it works in OP's case, but the reason why is subtle to new people using this as a reference. – Nissie

Nic, can you support that 'has to be an array on the stack' claim with a cite? – Neilla

Answer 4

I would say snprintf() is much more better until I read this passage:

https://buildsecurityin.us-cert.gov/bsi/articles/knowledge/coding/838-BSI.html

Short summary is: snprintf() not portable its behaviour change from system to system. The most serious problem with snprintf() can occur when snprintf() is implemented simply by calling sprintf().You may think it protects you from buffer overflow and let your guard down, but it may not.

So now I am still saying snprintf() safer but also being cautious when I use it.

Reseau

snprintf is specified in ISO C 99, which was 14 years before this answer, and 24 before today. The 2007-dated reference cited is correctly stating that snprintf is not in C90 and there have been different variants of it. But ... so what? – Pinion

Answer 5

There's an important difference between these two -- the snprintf call will scan the name argument to the end (terminating NUL) in order to figure out the correct return value. The sprintf call on the other hand will read AT MOST 255 characters from name.

So if name is a pointer to a non-NUL terminated buffer with at least 255 characters, the snprintf call might run off the end of the buffer and trigger undefined behavior (such as crashing), while the sprintf version will not.

Frederick

Answer 6

Your sprintf statement is correct, but I'd not be self-confident enough to use that for safety purpose (e.g. missing one cryptic char and you're shieldless) while there is snprintf around that can be applied to any format ... oh wait snprintf is not in ANSI C. It is (only?) C99. That could be a (weak) reason to prefer the other one.

Well. You could use strncpy, too, couldn't you ?

e.g.

  char buffer[MAX_LENGTH+1];
  buffer[MAX_LENGTH]=0;             // just be safe in case name is too long
  strncpy(buffer,MAX_LENGTH,name);  // strncpy will never overwrite last byte

Hokanson

And are you sure that the %.*s is available in ANSI C? I've tried to find the specification, but could only find one (untrustworthy) reference that did not specify the .*. – Sharmainesharman

@Eli: Specifying the precision (as here) or the field width as an asterisk has been in ANSI C since the original standard (1989/1990). snprintf() was added in the C99 standard. – Othella

@Michael - for string (%s) also? That's good to know, thanks. – Sharmainesharman

No, strncpyis not what he wants, search for strlcpy here on SO to see why. For example: stackoverflow.com/questions/2114896/… – Marci

@quinmars: that's a long babble from AndreyT you point me at here... If i try to summarize, the bad thing with strncpy is that it might not put a trailing '\0' if source string is too long, right ? that sounds easy enough to overcome to me (see updated code in my post). Now that might be felt "unsafe" because missing the additional "buffer[N]=0" will leave you unshielded. Or is there anything else I missed ? – Hokanson

Answer 7

So this was the first stack overflow related question that came up when searching for sprintf vs snprintf. So I figured this was the best place to add this answer.

It seems it's not an option to use sprintf on OSX 13.3 now because I'm getting these warnings.

'sprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror,-Wdeprecated-declarations]

So I went through and converted them all to

snprintf(char * restrict str, size_t size, const char * restrict format, ...);

It was pretty straight forward and it's always safer to use snprintf vs sprintf.

Gavette

Answer 8

Both will give the result you want, but snprintf is more generic, and will protect your string from overruns no matter the format string given.

In addition, because snprintf (or sprintf for that matter) adds a final \0, you should make the string buffer one byte bigger, char buff[MAXLEN + 1].

Sharmainesharman

That turns out not to be the case. From the snprintf doc: "The functions snprintf() and vsnprintf() write at most size bytes (including the terminating null byte ('\0')) to str." – Demonstration