Re: Implement C11 annex K?

[dalias <dalias at libc dot org>]

On Tue, Aug 12, 2014 at 07:15:10PM -0400, David A. Wheeler wrote:
> On Tue, 12 Aug 2014 18:08:11 -0400, Rich Felker <dalias@libc.org> wrote:
> > 1. Since the default constraint handler is implementation-defined,
> >    portable code cannot rely on the default handler to catch dangerous
> >    conditions, nor can it rely on the default handler to return an
> >    error status to the caller in a way that the caller can handle.
> 
> Portable application code can just set the handler, once.

Not necessarily. There could already be library code and multiple
threads running before entering main due to global ctors.

> Portable *library* code just needs to check for error returns; if they don't return,
> then the application author decided to do something else.  There's no state-saving necessary.

Such library code will not be safe in an application that didn't set a
handler, though. Consider the case where application A used library X,
which uses library Y as an implementation detail, and where library Y
uses annex K functions. Application A has no idea that annex K is in
use, and wants nothing to do with annex K, so it's not setting a
constraint handler. Library Y expects that the application using it
setup the constraint handler, but the application is not the "user" of
library Y; library X is the user. And library X can't setup the
handler because it's a library. The best library X could do is
document that it's using library Y, and that library Y requires a
proper annex K constraint handler to be setup, as part of its public
interface contract. But this is ugly because use of library Y should
be an implementation detail (it may even be optional at build time,
and may even be more indirect, via other libs) not something public.

> I think any libc implementation should by default just cause errors to be
> returned, since it has no way of knowing exactly what to do with the application instead.

This is certainly the proper behavior from a library-safety
standpoint, but it's also the least secure, since failure to check
return values could allow bugs to propagate. I suspect the historical
(Windows) usage of these interfaces mostly expects the constraint
handler to abort the program like an assertion failure.

To me, this is the worst-designed aspect of annex K: depending on how
you setup the runtime constraint handler, the functions have very
different usages:

1. If the constraint handler allows failure to be reported, their
   usage is much like strlcpy: they're bounded and valid programs can
   use them in ways that "would overflow" by relying on them to catch
   the overflow and convert it into an error case that can be handled.

2. If the constraint handler aborts, their usage is like an ugly,
   explicit version of _FORTIFY_SOURCE: they're assertion-checking
   functions that convert would-be UB into a predictable crash. Valid
   programs ensure that this never happens, but the check is there as
   an added hardening/safety measure.

> > As for your specific example, which I assume is strncpy_s,
> 
> Yes.
> 
> > the standard function which already solves this problem without the
> > additonal confusing and error-prone size arguments is snprintf:
> > snprintf(dest, destsize, "%s", source).
> 
> snprintf is botched, too.  If the source is as least destsize in length,
> then dest is not \0-terminated.  This is possible to detect, but off-by-one errors
> leading to vulnerabilities are extremely easy to do.

No, this is false. snprintf always null-terminates except when n is
zero, in which case it returns the length of the result without
writing anything.

> There are also a lot of annoyances with using snprintf this way.
> Why do I have to interpret a format string just to copy source to dest?
> Complete botch, complete fail.  String copying is an EXTREMELY common
> operation, it should be easy to do.

Straight string copying into a pre-existing buffer is usually a bug.
snprintf allows it as a degenerate special-case of the non-buggy,
highly-clean-and-efficient usage pattern of constructing a string in a
single opertion rather than via "Schlemiel the painter". In cases
where you just want a copy of an input string to keep past the
lifetime of (or at least the limits of your contract to access) the
original object, strdup is the right tool.

> > And the nonstandard but widely
> > available function that's even more convenient is strlcpy.
> 
> True.   strlcpy should be in glibc, too.  Lots of people use them.
> 
> My theory is that annex K is at least a formal spec, so it has an even *stronger*
> argument for inclusion.  But if people would rather start with strlcpy/strlcat, that's fine.

I'd much rather see strlcpy/strlcat added. Even though I consider them
a broken idiom, they're at least easy to use and provide a formulatic
fix for buggy legacy code, whereas the annex K functions are complex
to use (multiple, often redundant, size arguments, which can easily be
transposed by the programmer) and plagued by the constraint handler
issues.

Rich