Bad APIs at a glance
strcpy — Too easy to create a buffer overrun
strcat – Too easy to create a buffer overrun
strncpy – Deceptive. Doesn’t always NUL terminate!
strncat – Deceptive. Doesn’t always NUL terminate!
wstrcpy – Too easy to create a buffer overrun
wstrcat – Too easy to create a buffer overrun
wstrncpy – Deceptive. Doesn’t always NUL terminate!
wstrncat – Deceptive. Doesn’t always NUL terminate!
sprintf – Difficult to avoid buffer overruns with complex format strings
vsprintf – Difficult to avoid buffer overruns with complex format strings
wsprintf – Difficult to avoid buffer overruns with complex format strings
gets – Impossible to be safe with gets
strtok – Not reentrant
The problem
There is no way to limit the number of characters copied to the
destination buffer. Therefore there is potential for a buffer overflow
when the source has a length greater than that of destination buffer.
In the first code example below, the stack buffer aDest can be
overrun if GetString returns a string longer than 9 characters. If an
attacker can manipulate what GetString returns, perhaps because it is
read out of a network protocol or out of a multimedia file, they can
then manipulate the call stack. In the worst case they can cause a jump
to any address on the device by overwriting the return address that is
stored on the stack.
Strncpy, strncat
strncpy is deceptive
Strncpy will never copy past the end of the buffer given it which is
good. However if the input won’t fit into the buffer it will fill the
buffer and NOT terminate it with a NUL. Your string won’t be what you
think it is and code reading the string will read off the end of the
buffer. std_strlcpy will always NUL terminate whether or not the
destination buffer is large enough.
These comments apply to strncat, wstrncpy and wstrncat too.
gets
Gets has no way to bound how much is writes and worse what it writes
by definition comes from an external source. This is probably the most
dangerous function of the lot. Fortunately it is not widely used.
sprintf
Why sprintf is bad
There is no way to limit the number of characters written to the
destination. Therefore there is potential of buffer overflow when the
resulting output has a length greater than that of destination buffer.
strtok
Why strtok is bad
This function maintains internal state as a global variable. Because
of that it is not re entrant and not safe in a multi-threaded
environment. Calls to it from one thread to parse one string may
interleave with calls to it from another thread to parse another
string. The result will be that pointers one caller will get a pointer
to the other callers string.
Contemporary code is multi-threaded and thus it is not safe to use this. A bug
caused by the use of this function will be very difficult to track down
because it could be in code completely unrelated to the code
manifesting the problem.
What to do instead
The function strtok_r is safe because the caller passed storage for
the single pointer it can use to maintain the state it needs. Note that
strtok_r should always be checked for a NULL return value.
Scanf
This discussion applies to sscanf as well as scanf.
Why scanf is bad
If you use %s with scanf without a field length qualifier there is
no limit on how much data it will write to the buffer for the token
matching %s.
Also, as with any form of printf, the format string should be
constant, or if not constant, unmodifiable from out side the software
or the device. If the attacker can change the format string they can
trivially overrun a buffer.
Using scanf safely
Always specify a length for %s.
Always make the format string a compile time static const string (just use strings in quote marks like "%d %d").
In general scanf is cumbersome and error prone for parsing input.
Often other parsing approaches are better. If all you need is string to
integer conversion, strtoul is better.
[tags]bad standard apis, bad apis, dangerous standard APIs[/tags]
Popularity: 8%
