This is inspired by this question and the comments on one particular answer in that I learnt that strncpy
is not a very safe string handling function in C and that it pads zeros, until it reaches n
, something I was unaware of.
Specifically, to quote R..
strncpy does not null-terminate, and does null-pad the whole remainder of the destination buffer, which is a huge waste of time. You can work around the former by adding your own null padding, but not the latter. It was never intended for use as a "safe string handling" function, but for working with fixed-size fields in Unix directory tables and database files. snprintf(dest, n, "%s", src) is the only correct "safe strcpy" in standard C, but it's likely to be a lot slower. By the way, truncation in itself can be a major bug and in some cases might lead to privilege elevation or DoS, so throwing "safe" string functions that truncate their output at a problem is not a way to make it "safe" or "secure". Instead, you should ensure that the destination buffer is the right size and simply use strcpy (or better yet, memcpy if you already know the source string length).
And from Jonathan Leffler
Note that strncat() is even more confusing in its interface than strncpy() - what exactly is that length argument, again? It isn't what you'd expect based on what you supply strncpy() etc - so it is more error prone even than strncpy(). For copying strings around, I'm increasingly of the opinion that there is a strong argument that you only need memmove() because you always know all the sizes ahead of time and make sure there's enough space ahead of time. Use memmove() in preference to any of strcpy(), strcat(), strncpy(), strncat(), memcpy().
So, I'm clearly a little rusty on the C standard library. Therefore, I'd like to pose the question:
What C standard library functions are used inappropriately/in ways that may cause/lead to security problems/code defects/inefficiencies?
In the interests of objectivity, I have a number of criteria for an answer:
- Please, if you can, cite design reasons behind the function in question i.e. its intended purpose.
- Please highlight the misuse to which the code is currently put.
- Please state why that misuse may lead towards a problem. I know that should be obvious but it prevents soft answers.
Please avoid:
- Debates over naming conventions of functions (except where this unequivocably causes confusion).
- "I prefer x over y" - preference is ok, we all have them but I'm interested in actual unexpected side effects and how to guard against them.
As this is likely to be considered subjective and has no definite answer I'm flagging for community wiki straight away.
I am also working as per C99.
Any of the functions that manipulate global state, like
gmtime()
orlocaltime()
. These functions simply can't be used safely in multiple threads.EDIT:
rand()
is in the same category it would seem. At least there are no guarantees of thread-safety, and on my Linux system the man page warns that it is non-reentrant and non-threadsafe.I'm gonna go with the obvious :
With its remarkable particularity that it's simply impossible to use it appropriately.
On a wholly different tack, I've never really understood the benefits of
atan()
when there isatan2()
. The difference is thatatan2()
takes two arguments, and returns an angle anywhere in the range -π..+π. Further, it avoids divide by zero errors and loss of precision errors (dividing a very small number by a very large number, or vice versa). By contrast, theatan()
function only returns a value in the range -π/2..+π/2, and you have to do the division beforehand (I don't recall a scenario whereatan()
could be used without there being a division, short of simply generating a table of arctangents). Providing 1.0 as the divisor foratan2()
when given a simple value is not pushing the limits.I would put
printf
andscanf
pretty high up on this list. The fact that you have to get the formatting specifiers exactly correct makes these functions tricky to use and extremely easy to get wrong. It's also very hard to avoid buffer overruns when reading data out. Moreover, the "printf format string vulnerability" has probably caused countless security holes when well-intentioned programmers specify client-specified strings as the first argument to printf, only to find the stack smashed and security compromised many years down the line.One of my bêtes noire is
strtok()
, because it is non-reentrant and because it hacks the string it is processing into pieces, inserting NUL at the end of each token it isolates. The problems with this are legion; it is distressingly often touted as a solution to a problem, but is as often a problem itself. Not always - it can be used safely. But only if you are careful. The same is true of most functions, with the notable exception ofgets()
which cannot be used safely.Some of this functions are modifying some global state. (In windows) this state is shared per single thread - you can get unexpected result. For example, the first call of
rand
in every thread will give the same result, and it requires some care to make it pseudorandom, but deterministic (for debug purposes).