What are the C functions from the standard library that must / should be avoided ?

Question

I've read on stackoverflow that some C functions are 'obsolete' or should be 'avoided'. Can you please give me some examples of this kind of functions + the reason why.

What alternatives of those function exists ?

Can we use them safely - any good practices ?

Later edit: From the answers:

• gets : can cause buffer overflows.
• scanf : for strings it would be better to use fgets (followed by self parsing using sscanf or strchr).
• sprintf (instead use snprintf)
• strcat : avoid it when dealing with user input (instead use strncat, even if strncat has its pitfalls.)
• strcpy : avoid it when dealing with user input (instead use strncpy, even if strncopy has its pitfalls.)
• strtok : is not thread safe.

Answer 1

Avoid

• strtok for multithreaded programs as its not thread-safe.
• gets as it could cause buffer overflow

Answer 2

Deprecated Functions
Insecure
A perfect example of such a function is gets(), because there is no way to tell it how big the destination buffer is. Consequently, any program that reads input using gets() has a buffer overflow vulnerability. For similar reasons, one should use strncpy() in place of strcpy() and strncat() in place of strcat().

Yet some more examples include the tmpfile() and mktemp() function due to potential security issues with overwriting temporary files and which are superseded by the more secure mkstemp() function.

Non-Reentrant
Other examples include gethostbyaddr() and gethostbyname() which are non-reentrant (and, therefore, not guaranteed to be threadsafe) and have been superseded by the reentrant getaddrinfo() and freeaddrinfo().

You may be noticing a pattern here... either lack of security (possibly by failing to include enough information in the signature to possibly implement it securely) or non-reentrance are common sources of deprecation.

Outdated, Non-Portable
Some other functions simply become deprecated because they duplicate functionality and are not as portable as other variants. For example, bzero() is deprecated in favor of memset().

Thread Safety and Reentrance
You asked, in your post, about thread safety and reentrance. There is a slight difference. A function is reentrant if it does not use any shared, mutable state. So, for example, if all the information it needs is passed into the function, and any buffers needed are also passed into the function (rather than shared by all calls to the function), then it is reentrant. That means that different threads, by using independent parameters, do not risk accidentally sharing state. Reentrancy is a stronger guarantee than thread safety. A function is thread safe if it can be used by multiple threads concurrently. A function is thread safe if:

• It is reentrant (i.e. it does not share any state between calls), or:
• It is non-reentrant, but it uses synchronization/locking as needed for shared state.

In general, in the Single UNIX Specification and IEEE 1003.1 (i.e. "POSIX"), any function which is not guaranteed to be reentrant is not guaranteed to be thread safe. So, in other words, only functions which are guaranteed to be reentrant may be portably used in multithreaded applications (without external locking). That does not mean, however, that implementations of these standards cannot choose to make a non-reentrant function threadsafe. For example, Linux frequently adds synchronization to non-reentrant functions in order to add a guarantee (beyond that of the Single UNIX Specification) of threadsafety.

Strings (and Memory Buffers, in General)
You also asked if there is some fundamental flaw with strings/arrays. Some might argue that this is the case, but I would argue that no, there is no fundamental flaw in the language. C and C++ require you to pass the length/capacity of an array separately (it is not a ".length" property as in some other languages). This is not a flaw, per-se. Any C and C++ developer can write correct code simply by passing the length as a parameter where needed. The problem is that several APIs that required this information failed to specify it as a parameter. Or assumed that some MAX_BUFFER_SIZE constant would be used. Such APIs have now been deprecated and replaced by alternative APIs that allow the array/buffer/string sizes to be specified.

Scanf (In Answer to Your Last Question)
Personally, I use the C++ iostreams library (std::cin, std::cout, the << and >> operators, std::getline, std::istringstream, std::ostringstream, etc.), so I do not typically deal with that. If I were forced to use pure C, though, I would personally just use fgetc() or getchar() in combination with strtol(), strtoul(), etc. and parse things manually, since I'm not a huge fan of varargs or format strings. That said, to the best of my knowledge, there is no problem with [f]scanf(), [f]printf(), etc. so long as you craft the format strings yourself, you never pass arbitrary format strings or allow user input to be used as format strings, and you use the formatting macros defined in <inttypes.h> where appropriate. (Note, snprintf() should be used in place of sprintf(), but that has to do with failing to specify the size of the destination buffer and not the use of format strings). I should also point out that, in C++, boost::format provides printf-like formatting without varargs.

Answer 3

Almost any function that deals with NUL terminated strings is potentially unsafe. If you are receiving data from the outside world and manipulating it via the str*() functions then you set yourself up for catastrophe

Answer 4

Some people would claim that strcpy and strcat should be avoided, in favor of strncpy and strncat. This is somewhat subjective, in my opinion.

They should definitely be avoided when dealing with user input - no doubt here.

In code "far" from the user, when you just know the buffers are long enough, strcpy and strcat may be a bit more efficient because computing the n to pass to their cousins may be superfluous.

Answer 5

All functions that have a n-version should be avoided. Use strncpy instead and not strcpy for example.

Answer 6

Several answers here suggest using strncat() over strcat(); I'd suggest that strncat() (and strncpy()) should also be avoided. It has problems that make it difficult to use correctly and lead to bugs:

• the length parameter to strncat() is related to (but not quite exactly - see the 3rd point) the maximum number of characters that can be copied to the destination rather than the size of the destination buffer. This makes strncat() more difficult to use than it should be, particularly if multiple items will be concatenated to the destination.
• it can be difficult to determine if the result was truncated (which may or may not be important)
• it's easy to have an off-by-one error. As the C99 standard notes, "Thus, the maximum number of characters that can end up in the array pointed to by s1 is strlen(s1)+n+1" for a call that looks like strncat( s1, s2, n)

strncpy() also has an issue that can result in bugs you try to use it in an intuitive way - it doesn't guarantee that the destination is null terminated. To ensure that you have to make sure you specifically handle that corner case by dropping a '\0' in the buffer's last location yourself (at least in certain situations).

I'd suggest using something like OpenBSD's strlcat() and strlcpy() (though I know that some people dislike those functions; I believe they're far easier to use safely than strncat()/strncpy()).

Here's a little of what Todd Miller and Theo de Raadt had to say about problems with strncat() and strncpy():

There are several problems encountered when strncpy() and strncat() are used as safe versions of strcpy() and strcat(). Both functions deal with NUL-termination and the length parameter in different and non-intuitive ways that confuse even experienced programmers. They also provide no easy way to detect when truncation occurs. ... Of all these issues, the confusion caused by the length parameters and the related issue of NUL-termination are most important. When we audited the OpenBSD source tree for potential security holes we found rampant misuse of strncpy() and strncat(). While not all of these resulted in exploitable security holes, they made it clear that the rules for using strncpy() and strncat() in safe string operations are widely misunderstood.

OpenBSD's security audit found that bugs with these functions were "rampant". Unlike gets(), these functions can be used safely, but in practice there are a lot of problems because the interface is confusing, unintuitive and difficult to use correctly. I know that Microsoft has also done analysis (though I don't know how much of their data they may have published), and as a result have banned (or at least very strongly discouraged - the 'ban' might not be absolute) the use of strncat() and strncpy() (among other functions).

Some links with more information:

• http://www.usenix.org/events/usenix99/full_papers/millert/millert_html/
• http://en.wikipedia.org/wiki/Off-by-one_error#Security_implications
• http://blogs.msdn.com/michael_howard/archive/2004/10/29/249713.aspx
• http://blogs.msdn.com/michael_howard/archive/2004/11/02/251296.aspx
• http://blogs.msdn.com/michael_howard/archive/2004/12/10/279639.aspx
• http://blogs.msdn.com/michael_howard/archive/2006/10/30/something-else-to-look-out-for-when-reviewing-code.aspx

Answer 7

Don't forget about sprintf - it is the cause of many problems. This is true because the alternative, snprintf has sometimes different implementations which can make you code unportable.

1. linux: http://linux.die.net/man/3/snprintf

2. windows: http://msdn.microsoft.com/en-us/library/2ts7cx93%28VS.71%29.aspx

In case 1 (linux) the return value is the amount of data needed to store the entire buffer (if it is smaller than the size of the given buffer then the output was truncated)

In case 2 (windows) the return value is a negative number in case the output is truncated.

Generally you should avoid functions that are not:

1. buffer overflow safe (a lot of functions are already mentioned in here)

2. thread safe/not reentrant (strtok for example)

In the manual of each functions you should search for keywords like: safe, sync, async, thread, buffer, bugs

Answer 8

Once again people are repeating, mantra-like, the ludicrous assertion that the "n" version of str functions are safe versions.

If that was what they were intended for then they would always null terminate the strings.

The "n" versions of the functions were written for use with fixed length fields (such as directory entries in early file systems) where the nul terminator is only required if the string does not fill the field. This is also the reason why the functions have strange side effects that are pointlessly inefficient if just used as replacements - take strncpy() for example:

If the array pointed to by s2 is a string that is shorter than n bytes, null bytes are appended to the copy in the array pointed to by s1, until n bytes in all are written.

As buffers allocated to handle filenames are typically 4kbytes this can lead to a massive deterioration in performance.

If you want "supposedly" safe versions then obtain - or write your own - strl routines (strlcpy, strlcat etc) which always nul terminate the strings and don't have side effects. Please note though that these aren't really safe as they can silently truncate the string - this is rarely the best course of action in any real-world program. There are occasions where this is OK but there are also many circumstances where it could lead to catastrophic results (e.g. printing out medical prescriptions).

Answer 9

It is very hard to use scanf safely. Good use of scanf can avoid buffer overflows, but you are still vulnerable to undefined behavior when reading numbers that don't fit in the requested type. In most cases, fgets followed by self-parsing (using sscanf, strchr, etc.) is a better option.

But I wouldn't say "avoid scanf all the time". scanf has its uses. As an example, let's say you want to read user input in a char array that's 10 bytes long. You want to remove the trailing newline, if any. If the user enters more than 9 characters before a newline, you want to store the first 9 characters in the buffer and discard everything until the next newline. You can do:

char buf[10];
scanf("%9[^\n]%*[^\n]", buf));
getchar();

Once you get used to this idiom, it's shorter and in some ways cleaner than:

char buf[10];
if (fgets(buf, sizeof buf, stdin) != NULL) {
    char *nl;
    if ((nl = strrchr(buf, '\n')) == NULL) {
        int c;
        while ((c = getchar()) != EOF && c != '\n') {
            ;
        }
    } else {
        *nl = 0;
    }
}

Answer 10

It is probably worth adding again that strncpy() is not the general-purpose replacement for strcpy() that it's name might suggest. It is designed for fixed-length fields that don't need a nul-terminator (it was originally designed for use with UNIX directory entries, but can be useful for things like encryption key fields).

It is easy, however, to use strncat() as a replacement for strcpy():

if (dest_size > 0)
{
    dest[0] = '\0';
    strncat(dest, source, dest_size - 1);
}

(The if test can obviously be dropped in the common case, where you know that dest_size is definitely nonzero).

Answer 11

atoi is not thread safe. I use strtol instead, per recommendation from the man page.

Answer 12

Also check out Microsoft's list of banned APIs. These are APIs (including many already listed here) that are banned from Microsoft code because they are often misused and lead to security problems.

You may not agree with all of them, but they are all worth considering. They add an API to the list when its misuse has led to a number of security bugs.