[ACCEPTED]-What makes a C standard library function dangerous, and what is the alternative?-function

Accepted answer
Score: 48

In the old days, most of the string functions 9 had no bounds checking. Of course they couldn't 8 just delete the old functions, or modify 7 their signatures to include an upper bound, that 6 would break compatibility. Now, for almost 5 every one of those functions, there is an 4 alternative "n" version. For example:

strcpy -> strncpy
strlen -> strnlen
strcmp -> strncmp
strcat -> strncat
strdup -> strndup
sprintf -> snprintf
wcscpy -> wcsncpy
wcslen -> wcsnlen

And 3 more.

See also https://github.com/leafsr/gcc-poison which is a project to create 2 a header file that causes gcc to report 1 an error if you use an unsafe function.

Score: 35

Yes, fgets(..., ..., STDIN) is a good alternative to gets(), because 55 it takes a size parameter (gets() has in fact 54 been removed from the C standard entirely 53 in C11). Note that fgets() is not exactly a drop-in 52 replacement for gets(), because the former will 51 include the terminating \n character if there 50 was room in the buffer for a complete line 49 to be read.

scanf() is considered problematic in 48 some cases, rather than straight-out "bad", because 47 if the input doesn't conform to the expected 46 format it can be impossible to recover sensibly 45 (it doesn't let you rewind the input and 44 try again). If you can just give up on 43 badly formatted input, it's useable. A 42 "better" alternative here is to use an input 41 function like fgets() or fgetc() to read chunks of input, then 40 scan it with sscanf() or parse it with string handling 39 functions like strchr() and strtol(). Also see below for 38 a specific problem with the "%s" conversion 37 specifier in scanf().

It's not a standard C function, but 36 the BSD and POSIX function mktemp() is generally 35 impossible to use safely, because there 34 is always a TOCTTOU race condition between 33 testing for the existence of the file and 32 subsequently creating it. mkstemp() or tmpfile() are good 31 replacements.

strncpy() is a slightly tricky function, because 30 it doesn't null-terminate the destination 29 if there was no room for it. Despite the 28 apparently generic name, this function was 27 designed for creating a specific style of 26 string that differs from ordinary C strings 25 - strings stored in a known fixed width 24 field where the null terminator is not required 23 if the string fills the field exactly (original 22 UNIX directory entries were of this style). If 21 you don't have such a situation, you probably 20 should avoid this function.

atoi() can be a bad 19 choice in some situations, because you can't 18 tell when there was an error doing the conversion 17 (e.g., if the number exceeded the range 16 of an int). Use strtol() if this matters to you.

strcpy(), strcat() and 15 sprintf() suffer from a similar problem to gets() - they 14 don't allow you to specify the size of the 13 destination buffer. It's still possible, at 12 least in theory, to use them safely - but 11 you are much better off using strncat() and snprintf() instead 10 (you could use strncpy(), but see above). Do note 9 that whereas the n for snprintf() is the size of the 8 destination buffer, the n for strncat() is the maximum 7 number of characters to append and does 6 not include the null terminator. Another 5 alternative, if you have already calculated 4 the relevant string and buffer sizes, is 3 memmove() or memcpy().

On the same theme, if you use the scanf() family 2 of functions, don't use a plain "%s" - specify 1 the size of the destination e.g. "%200s".

Score: 20

strtok() is generally considered to be evil because 2 it stores state information between calls. Don't 1 try running THAT in a multithreaded environment!

Score: 11

Strictly speaking, there is one really dangerous 49 function. It is gets() because its input is not 48 under the control of the programmer. All 47 other functions mentioned here are safe 46 in and of themselves. "Good" and "bad" boils 45 down to defensive programming, namely preconditions, postconditions 44 and boilerplate code.

Let's take strcpy() for example. It 43 has some preconditions that the programmer 42 must fulfill before calling the function. Both 41 strings must be valid, non-NULL pointers 40 to zero terminated strings, and the destination 39 must provide enough space with a final string 38 length inside the range of size_t. Additionally, the 37 strings are not allowed to overlap.

That 36 is quite a lot of preconditions, and none 35 of them is checked by strcpy(). The programmer must 34 be sure they are fulfilled, or he must explicitly 33 test them with additional boilerplate code 32 before calling strcpy():

n = DST_BUFFER_SIZE;
if ((dst != NULL) && (src != NULL) && (strlen(dst)+strlen(src)+1 <= n))
{
    strcpy(dst, src);
}

Already silently assuming 31 the non-overlap and zero-terminated strings.

strncpy() does 30 include some of these checks, but it adds 29 another postcondition the programmer must 28 take care for after calling the function, because 27 the result may not be zero-terminated.

strncpy(dst, src, n);
if (n > 0)
{
    dst[n-1] = '\0';
}

Why 26 are these functions considered "bad"? Because 25 they would require additional boilerplate 24 code for each call to really be on the safe 23 side when the programmer assumes wrong about 22 the validity, and programmers tend to forget 21 this code.

Or even argue against it. Take 20 the printf() family. These functions return a status 19 that indicate error and success. Who checks 18 if the output to stdout or stderr succeeded? With 17 the argument that you can't do anything 16 at all when the standard channels are not 15 working. Well, what about rescuing the user 14 data and terminating the program with an 13 error-indicating exit code? Instead of the 12 possible alternative of crash and burn later 11 with corrupted user data.

In a time- and 10 money-limited environment it is always the 9 question of how much safety nets you really 8 want and what is the resulting worst case 7 scenario? If it is a buffer overflow as 6 in case of the str-functions, then it makes 5 sense to forbid them and probably provide 4 wrapper functions with the safety nets already 3 within.

One final question about this: What 2 makes you sure that your "good" alternatives 1 are really good?

Score: 7

Any function that does not take a maximum 4 length parameter and instead relies on an 3 end-of- marker to be present (such as many 2 'string' handling functions).

Any method 1 that maintains state between calls.

Score: 6
  • sprintf is bad, does not check size, use snprintf
  • gmtime, localtime -- use gmtime_r, localtime_r

0

Score: 4

To add something about strncpy most people 7 here forgot to mention. strncpy can result 6 in performance problems as it clears the 5 buffer to the length given.

char buff[1000];
strncpy(buff, "1", sizeof buff);

will copy 1 char 4 and overwrite 999 bytes with 0

Another reason 3 why I prefer strlcpy (I know strlcpy is 2 a BSDism but it is so easy to implement 1 that there's no excuse to not use it).

Score: 3

View page 7 (PDF page 9) SAFECode Dev Practices

Edit: From the 3 page -

strcpy family
strncpy family
strcat 2 family
scanf family
sprintf family
gets 1 family

Score: 2

strcpy - again!

Most people agree that strcpy is 11 dangerous, but strncpy is only rarely a 10 useful replacement. It is usually important 9 that you know when you've needed to truncate 8 a string in any case, and for this reason 7 you usually need to examine the length of 6 the source string anwyay. If this is the 5 case, usually memcpy is the better replacement 4 as you know exactly how many characters 3 you want copied.

e.g. truncation is error:

n = strlen( src );

if( n >= buflen )
    return ERROR;

memcpy( dst, src, n + 1 );

truncation 2 allowed, but number of characters must be 1 returned so caller knows:

n = strlen( src );

if( n >= buflen )
    n = buflen - 1;

memcpy( dst, src, n );
dst[n] = '\0';

return n;

More Related questions