Re: Usage of C11 Annex K Bounds-checking interfaces on GCC

[Jeffrey Walton <noloader@gmail.com>]

On Sun, Dec 15, 2019 at 1:25 PM David Brown <david.brown@hesbynett.no> wrote:
>
> On 15/12/2019 02:57, Jeffrey Walton wrote:
> > On Sat, Dec 14, 2019 at 12:36 PM Martin Sebor <msebor@gmail.com> wrote:
> >>
> >> On 12/9/19 8:15 PM, li zi wrote:
> >>> Hi All,
> >>> We are using gcc in our projects and we found some of the C standard functions (like memcpy, strcpy) used in gcc may induce security vulnerablities like buffer overflow. Currently we have not found any instances which causes such issues.
>
> (This post is all "In My Humble Opinion".)
>
> The correct use of memcpy, strcpy, etc., does not introduce any security
> vulnerabilities.  /Incorrect/ use does - just as incorrect use of any
> function can risk bugs, and therefore security vulnerabilities.
>
> >>> But we feel better to change these calls to Cll Annex K Bounds-checking interfaces like memcpy_s, strcpy_s etc. By defining a secure calls method (list of func pointers) and allowing application to register the method. I understand that this affects performance because of return value check added for xxxx_s calls, but this will relieve overflow kind of issues from code. And also currently using bounds-checking interfaces is a general industry practice.
> >>> Please share your opinion on it, and if any discussion happened in community to do some changes in future.
>
> Thoughtless "change all standard <string.h> functions to Annex K
> functions" is management arse-covering as an alternative to proper
> quality development techniques.  It adds nothing to the software
> quality, it has no reduction in the risk of errors or their
> consequences.  It is nothing more than a way to try to blame other
> people when something has gone wrong.

If RTFM was going to work, then it would have happened in the last 50
years or so.

If error free programming was going to happen, then it would have
happened in the last 50 years or so.

Come back to reality.

> If you find that using these functions really does give you better
> software with lower risks of problems, then by all means use them.  But
> don't use them blindly just because someone says they are "safer".

Hard to do when they are missing.

> >> GCC's Object Size Checking is a non-intrusive solution to
> >> the problem.  It avoids the considerable risk of introducing
> >> bugs while replacing existing calls with those to the _s
> >> functions.  The implementation is restricted to constant
> >> sizes so its effectiveness is a limited, but we have been
> >> discussing enhancing it to non-constant sizes as well, as
> >> Clang already does.  With that, it should provide protection
> >> with an effectiveness comparable to the _s functions but
> >> without any of the downsides.  (Note that GCC's buffer
> >> overflow warnings are not subject to the same limitation.)
> >>
> >> Besides Object Size Checking, I would suggest making use of
> >> the new attribute access.  It lets GCC detect (though not
> >> prevent) out-of-bounds accesses by calls to user-defined
> >> functions decorated with the attribute.
> >
> > The safer functions have three or four security goals. The workarounds
> > don't meet the goals.
>
> Let's call them "Annex K" functions, rather than "safer functions",
> until it is demonstrated that they actually /are/ safer in some way.
> And let's talk about other tools in the toolbox, to use in addition or
> as alternatives, rather than calling them "workarounds".

Microsoft calls them "safer" functions. They are "safer" then the
original C functions they are supplementing. For completeness,
Microsoft does not claim they are completely safe.

Runtime crashes due to unwanted terminates are a real problem if the
program handles sensitive data, like passwords and private keys. We
can't have crash dumps with passwords and private keys written to the
file system in a core dump, or shipped to an error reporting service
like Crash Reporter or Windows Error Reporting. That's a different
[policy] problem.

> > The safer functions require the destination size to ensure a buffer
> > overflow does not occur.
>
> That is useless unless you know the destination size.  And if you know
> the destination size, you can use that to make sure your calls to
> memcpy, strcpy, etc., are safe and correct.

Hugh? Are you begging the argument:

    char* ptr = malloc (50);

And then claiming you don't know the size?

> The use of the Annex K functions therefore gives you no technical
> benefits in this aspect.  They may, however, give you the non-technical
> benefit of forcing the programmer to think about destination sizes -
> they can't be lazy about it.  However, if you are dealing with code that
> needs to be of good quality then you should already have development
> practices that cover this - such as code reviews, programmer training,
> code standards, etc.

Developer training does not work. If it was going to work, then it
would have happened in the last 50 years or so.

Microsoft recognized the fact years ago. You have to force developers
to use something safer.

> > The safe functions always terminate the destination buffer.
>
> So do the standard functions when used correctly.

The standard functions _do not_ terminate the buffer under all
circumstances. Just look at the return value from printf and sprintf.
Termination only occurs when the return value is less than the buffer
size.

This sounds like the "RTFM is going to magically work" someday argument.

Can you provide a year this is going to happen? When are IQ's suddenly
going to rise?

> I'm quite happy to agree that strncpy has a silly specification, and it
> can surprise people both in its inefficiency and in the fact that it
> does not always null-terminate the destination.  Creating a "strncpy_s"
> with redundant parameters and confusingly different semantics is /not/
> the answer.  The right solution would be to deprecate strncpy, and make
> a replacement with a different name and better semantics, such as:
>
> char * strscpy(char * restrict s1, const char * restrict s2, size_t n)
> {
>      if (n > 0) {
>          *s1 = '\0';
>          strncat(s1, s2, n - 1);
>      }
>      return s1;
> }
>
> /That/ would have been a useful, clear, and consistent function that
> copies at most "n" characters from the string, and always terminates the
> copy.

I don't think a known dangerous and banned function is a good example.
strcpy() is banned by both Microsoft and APple. Only Linux still
embraces strcpy(). strcpy() still suffers the problem that is trying
to be corrected.

> > The safe functions provide a consistent return value. There is only
> > one success code.
>
> There is only one success code from the Annex K functions - but no
> guarantees about failures.  It will call the constraint handler - which
> might return, or might not.  Frankly, you have no idea what it might do
> in the general case, since the constraint handler is a global state
> variable and not even thread-safe.
>
> The only way to write good, safe and reliable code using the Annex K
> "_s" functions is to make sure they are called with sizes that can never
> cause run-time constraint errors.  And when you can do that, you can do
> it just as well with the standard library functions.
>
> >
> > It is easy to teach a novice user how to use the safe functions
> > correctly because there is only one rule for return values.
>
> But there are multiple and unclear parameters to the functions.  How is
> the constraint handler "easy to teach" ?

It is not clear (to me) how (1) the destination buffer and (2) the
buffer's size is confusing to users. Especially when every safer
function requires the parameters.

It seems (to me) the consistent parameters are easier to teach and learn.

> You'd to a lot better teaching people to think about the size of buffers
> that they use - both for the source and for the destination.  (The Annex
> K functions have mostly forgotten about checks on the size of the
> source.)  Teach them to think about these things correctly and it will
> work for /all/ code - teach them to use the "_s" functions and they'll
> just get a false sense of security without understanding the real issue.
>
> > The safer functions should provide portability across platforms. Write
> > once, run everywhere.
>
> That is a joke, surely.  Even if there were many C implementations that
> supported them (there aren't), even if they were efficient enough to be
> a sensible choice for important code (usually they are not), they are so
> full of fundamentally implementation-dependent behaviour that they are
> not portable.

No, that's serious.

I do a lot of cross-platform work. It would be awesome to have
something that actually works across all platforms. And it would be
awesome to see the same functions everywhere when auditing code.

> > Consider, if object size checking were a complete replacement, then
> > glibc should not make so many appearances on BugTraq for out-of-bound
> > reads and writes. Confer,
> > https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=glibc.
>
> Out of bounds accesses are a problem and a source of error in C coding -
> no one disputes that.  There is scope for improvement of the C standard
> str and mem functions, in terms of making them easier to use safely,
> making them harder to use incorrectly, and making them more efficient -
> I doubt if anyone will dispute that either.  But the Annex K functions
> are most certainly not the answer.

I generally consider the Glibc folks better trained in C and more
knowledgeable of the C standard then me. If the Glibc folks are making
the mistakes, then there is no hope in practice for folks like me or
those who are just starting in C. There are too many sharp edges.

Watching the Glibc folks make the mistakes should indicate there is a
critical problem that needs to be addressed.

I'll take a destination buffer size as a first step. And I do look for
how terminates are handled, and how set_terminate is used in code
audits.

Jeff