This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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 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".


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".

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.

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.

The safe functions always terminate the destination buffer.

So do the standard functions when used correctly.

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.


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" ?

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.


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.

The glibc folks have done more harm then good with their politics.


That is an opinion that is not universal, I think.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]