Which of sprintf/snprintf is more secure?

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.

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.

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.

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);

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.

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.

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

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].