Re: New static analysis results

[Rich Felker <dalias@libc.org>]

On Fri, Sep 05, 2014 at 10:50:52PM +0200, Jens Gustedt wrote:
> Am Freitag, den 05.09.2014, 14:53 -0400 schrieb Rich Felker:
> > See also asctime: it's even worse, specified to be UB, via potential
> > buffer overflow, if the values are outside of the expected range.
> > 
> > These functions really just should not be used for anything. Short of
> > rolling your own, strftime is the only correct way to format time as a
> > string.
> 
> the corresponding xxx_s functions from Annex K are a bit better, here.
> 
> > At some point it would be nice to make a big list of standard C
> > functions that are utterly unusable due to UB on errors. Unusable due
> > to lack of thread safety is another big area, too.
> 
> Annex K can basically be read as such a list (for C itself, not POSIX)
> and gives replacements for them, I think.

I don't think so. Out of Annex K, here are the functions that replace
a fundamentally unusable (due to UB) function with the same name minus
the final _s (5):

sprintf_s (but snprintf already did it better)
vsprintf_s (ditto)
gets_s (but gets was removed, and fgets already did it better)
asctime_s (but strftime already did it better)
ctime_s (ditto)

So from this first group, I would call them all utterly useless
replacements -- they're replacing things that already had better
replacements.

Second, the ones which replace a function that's unusable due to lack
of thread-safety (5):

strtok_s
strerror_s
wcstok_s
gmtime_s
localtime_s

For all of these, POSIX already replaced them with _r versions; the _s
versions are mostly gratuitous duplicates with the same or worse
interfaces. So they're "useful" only for plain-C without POSIX.

And next, the ones which replace a function that's usable in a
thread-safe manner, but lacks a context, necessitating the use of TLS
for context (2):

bsearch_s
qsort_s

These seem like welcome additions.

And finally, the ones which have nothing to do with fixing a problem
in the function they 'replace' (52):

tmpfile_s
tmpnam_s
fopen_s
freopen_s
fprintf_s
fscanf_s
printf_s
scanf_s
snprintf_s
sscanf_s
vfprintf_s
vfscanf_s
vprintf_s
vscanf_s
vsnprintf_s
vsscanf_s
getenv_s
wctomb_s
mbstowcs_s
wcstombs_s
memcpy_s
memmove_s
strcpy_s
strncpy_s
strcat_s
strncat_s
memset_s
strnlen_s
fwprintf_s
fwscanf_s
snwprintf_s
swprintf_s
swscanf_s
vfwprintf_s
vfwscanf_s
vsnwprintf_s
vswprintf_s
vswscanf_s
vwprintf_s
vwscanf_s
wprintf_s
wscanf_s
wcscpy_s
wcsncpy_s
wmemcpy_s
wmemmove_s
wcscat_s
wcsncat_s
wcsnlen_s
wcrtomb_s
mbsrtowcs_s
wcsrtombs_s

> Implementing these functions, using them with a constraint handler
> that is set to ignore_handler_s, and checking for the return values of
> the functions is a realistic alternative to all this UB stuff.

Setting constraint handler is not thread-safe or library-safe, so it's
not a practical solution.

Rich